expand chronyd_specify_remote_server to be aligned with CIS#14241
Conversation
These use cases are described for example in CIS for RHEL 10, section 2.3.2.
| EOF | ||
|
|
||
| # Create empty sources.d directory | ||
| mkdir -p /etc/chrony/sources.d |
There was a problem hiding this comment.
What if this directory already exists? I think the directory should be erased before.
| @@ -1,21 +1,107 @@ | |||
| <def-group> | |||
| <definition class="compliance" id="chronyd_specify_remote_server" version="1"> | |||
| <definition class="compliance" id="chronyd_specify_remote_server" version="2"> | |||
There was a problem hiding this comment.
This rule is a part of many profiles in many products. Do all of these profiles allow chrony to use sourcedir and confdir?
There was a problem hiding this comment.
Yes, this is a feature of Chrony. And it is supported for long time.
| <tt>sourcedir</tt> or <tt>confdir</tt> directives in <tt>{{{ chrony_conf_path }}}</tt>. | ||
| When using <tt>sourcedir</tt>, create <tt>.sources</tt> files in the specified directory: | ||
| <pre># In {{{ chrony_conf_path }}}: | ||
| sourcedir /etc/chrony/sources.d |
There was a problem hiding this comment.
I think that the remediations namely Ansible remediation should be improved so that they would remove problematic items from files in sourcedir or confdir.
ATEX Test ResultsTest artifacts have been submitted to Testing Farm. Results: View Test Results This comment was automatically generated by the ATEX workflow. |
|
|
||
| # Create conf.d directory and file | ||
| mkdir -p /etc/chrony/conf.d | ||
| echo "pool 1.pool.ntp.org" > /etc/chrony/conf.d/ntp-servers.conf |
There was a problem hiding this comment.
I think, that a good practice for writing tests for such rules would be to remove configuration entries from configs (including sub directories) that are unrelated to the current test. For example, clearing the content of /etc/chrony/conf.d before writing pool entry into the file will ensure that test is passing because pool entry exist in this particular file and not because it found similar entry somewhere else in the chrony_conf_path or /etc/chrony/conf.d.
|
@vojtapolasek: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
I've updated chronyd_specify_remote_server tests to remove configuration entries from configs (including subdirectories) that are unrelated to the current test (more information is in my comment). All of these tests are passing on local environment.
I've also updated the Ansible remediation to align with CIS and to be capable of being executed standalone. The failing tests are not caused by any code changes but rather infrastructure problems. Overall, I think this PR is ready for merge.
jan-cerny
left a comment
There was a problem hiding this comment.
I have reviewed the changes and I have run automatus test locally with both Bash and Ansible remediations.
Description:
confdirand 'sourcedir' directives from the main chrony config file and examining files within these directories forserverorpooldirectivesRationale:
Review Hints:
Use automatus.