Improve stability of timesyncd based remediation #11247
Conversation
teacup-on-rockingchair
added
Ansible
|
Start a new ephemeral environment with changes proposed in this pull request: sle12 (from CTF) Environment (using Fedora as testing environment) Fedora Testing Environment Oracle Linux 8 Environment |
|
This datastream diff is auto generated by the check Click here to see the full diffbash remediation for rule 'xccdf_org.ssgproject.content_rule_service_timesyncd_configured' differs.
--- xccdf_org.ssgproject.content_rule_service_timesyncd_configured
+++ xccdf_org.ssgproject.content_rule_service_timesyncd_configured
@@ -2,7 +2,6 @@
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && { rpm --quiet -q systemd; }; then
var_multiple_time_servers=''
-
IFS=',' read -r -a time_servers_array <<< "$var_multiple_time_servers"
preferred_ntp_servers_array=("${time_servers_array[@]:0:2}")
@@ -10,9 +9,9 @@
fallback_ntp_servers_array=("${time_servers_array[@]:2}")
fallback_ntp_servers=$( echo "${fallback_ntp_servers_array[@]}"|sed -e 's/\s\+/,/g' )
+IFS=" " mapfile -t current_cfg_arr < <(ls -1 /etc/systemd/timesyncd.d/* 2>/dev/null)
config_file="/etc/systemd/timesyncd.d/oscap-remedy.conf"
-current_cfg_arr=( "/etc/systemd/timesyncd.conf" )
-current_cfg_arr+=("$(ls /etc/systemd/timesyncd.d/*)")
+current_cfg_arr+=( "/etc/systemd/timesyncd.conf" )
# Comment existing NTP FallbackNTP settings
for current_cfg in "${current_cfg_arr[@]}"
do
ansible remediation for rule 'xccdf_org.ssgproject.content_rule_service_timesyncd_configured' differs.
--- xccdf_org.ssgproject.content_rule_service_timesyncd_configured
+++ xccdf_org.ssgproject.content_rule_service_timesyncd_configured
@@ -37,8 +37,8 @@
- name: Configure Systemd Timesyncd Servers - Set Fallback NTP Servers
ansible.builtin.set_fact:
- fallback_ntp_servers: '{{ var_multiple_time_servers.split(",") | slice(2)| last
- | join(",") }}'
+ fallback_ntp_servers: '{{ var_multiple_time_servers.split(",") | slice(2)| list
+ | last | join(",") }}'
when:
- ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
- '"systemd" in ansible_facts.packages'
bash remediation for rule 'xccdf_org.ssgproject.content_rule_service_timesyncd_root_distance_configured' differs.
--- xccdf_org.ssgproject.content_rule_service_timesyncd_root_distance_configured
+++ xccdf_org.ssgproject.content_rule_service_timesyncd_root_distance_configured
@@ -2,13 +2,15 @@
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && { rpm --quiet -q systemd; }; then
config_file="/etc/systemd/timesyncd.d/oscap-remedy.conf"
-current_cfg_arr=( "/etc/systemd/timesyncd.conf" )
-current_cfg_arr+=("$(ls /etc/systemd/timesyncd.d/*)")
-# Comment existing NTP RootDistance settings
-for current_cfg in "${current_cfg_arr[@]}"
-do
- sed -i 's/^RootDistanceMax/#&/g' "$current_cfg"
-done
+IFS=" " mapfile -t current_cfg_arr < <(ls -1 /etc/systemd/timesyncd.d/* 2>/dev/null)
+current_cfg_arr+=( "/etc/systemd/timesyncd.conf" )
+# Comment existing NTP RootDistanceMax settings
+if [ ${#current_cfg_arr[@]} -ne 0 ]; then
+ for current_cfg in "${current_cfg_arr[@]}"
+ do
+ sed -i 's/^RootDistanceMax/#&/g' "$current_cfg"
+ done
+fi
# Set RootDistance in drop-in configuration
echo "RootDistanceMax=1" >> "$config_file"
|
There was a problem hiding this comment.
The fails of Automatus on CS8 and CS9 are expected because the tested rules are marked as SLE 12 / SLE 15.
On the other hand, I don't like the fail of Automatus on SLE15. I think that the problem is that the /etc/systemd/timesyncd.d/ might not exist by default. I suggest adding a command that creates this directory to both common.sh files in both rules tests directory.
| sed -i 's/^NTP/#&/g' "$current_cfg" | ||
| sed -i 's/^FallbackNTP/#&/g' "$current_cfg" | ||
| done | ||
| if [ ${#current_cfg_arr[@]} -ne 0 ]; then |
There was a problem hiding this comment.
I think this condition is superfluous because the array will always have at least one element because of the assignment on line 16.
There was a problem hiding this comment.
Thanks - dropped in 669c80c
| done | ||
| IFS=" " mapfile -t current_cfg_arr < <(ls -1 /etc/systemd/timesyncd.d/* 2>/dev/null) | ||
| current_cfg_arr+=( "/etc/systemd/timesyncd.conf" ) | ||
| # Comment existing NTP FallbackNTP settings |
There was a problem hiding this comment.
The old comment seem to be correct, the code below comments out RootDistanceMax, not FallbackNTP.
There was a problem hiding this comment.
Thanks for the note changed in 7286f5f
Improve generating array of config files for timesyncd service Improve stability of ansible remediation jinja code to get last element. In some cases last element cannot be extracted directly and appears issue like pallets/jinja#897
… test environment Thanks to @jan-cerny for the suggestion
teacup-on-rockingchair
force-pushed
the
timesyncd_cfg_remediations_improve
branch
from
913e40b
to
5d5fd51
Compare
|
I can see that Automatus SLE15 is still failing, also, I think that the change has to be done in both rules |
…y is setup for test environment
|
Code Climate has analyzed commit 1fcf36b 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 58.8%. View more on Code Climate. |
Description:
Rationale:
Improve generating array of config files for timesyncd service , reworked reading array of configuration files to avoid problems with no or more than one line files
Improve stability of ansible remediation jinja code to get last element. In some cases last element cannot be extracted directly and appears issue like Possible inconsistency between
firstandlastfilters pallets/jinja#897