Skip to content

Fix chronyd_specify_remote_server remediation - #15100

Open
alanmcanonical wants to merge 9 commits into
ComplianceAsCode:masterfrom
alanmcanonical:chronyd_fix
Open

Fix chronyd_specify_remote_server remediation#15100
alanmcanonical wants to merge 9 commits into
ComplianceAsCode:masterfrom
alanmcanonical:chronyd_fix

Conversation

@alanmcanonical

Copy link
Copy Markdown
Contributor

Description:

  • Move the logic of checking existance inside for loop for bash_ensure_there_are_servers_in_ntp_compatible_config_file
  • Change the primary remediation logic to append if not exist
  • Skip chronyd conf dir tests on Ubuntu

Rationale:

  • Current remediation logic is "if server|pool [[:graph:]]+ not exists: run the macro, else: do nothing.
  • The macro logic is also weird: "if # server doesn't exist: append urls, else uncomment those lines. Its regex only matching #[[:space:]]*server instead of targeting server like #\s*server\s+0.pool.ntp.org

…there_are_servers_in_ntp_compatible_config_file

Signed-off-by: Alan Moore <alan.moore@canonical.com>
Signed-off-by: Alan Moore <alan.moore@canonical.com>
Signed-off-by: Alan Moore <alan.moore@canonical.com>
@openshift-ci openshift-ci Bot added the needs-ok-to-test Used by openshift-ci bot. label Sep 8, 2026
@openshift-ci

openshift-ci Bot commented Sep 8, 2026

Copy link
Copy Markdown

Hi @alanmcanonical. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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-sigs/prow repository.

@alanmcanonical alanmcanonical changed the title Chronyd fix Fix chronyd_specify_remote_server remediation Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff
bash remediation for rule 'xccdf_org.ssgproject.content_rule_chronyd_specify_remote_server' differs.
--- xccdf_org.ssgproject.content_rule_chronyd_specify_remote_server
+++ xccdf_org.ssgproject.content_rule_chronyd_specify_remote_server
@@ -6,17 +6,34 @@
 
 config_file="/etc/chrony.conf"
 
-if ! grep -q '^[[:space:]]*\(server\|pool\)[[:space:]]\+[[:graph:]]\+' "$config_file" ; then
-  if ! grep -q '#[[:space:]]*server' "$config_file" ; then
-    for server in $(echo "$var_multiple_time_servers" | tr ',' '\n') ; do
-      printf '\nserver %s' "$server" >> "$config_file"
-    done
-  else
-    sed -i 's/#[ \t]*server/server/g' "$config_file"
-  fi
-  if [[ -s "$config_file" ]] && [[ -n "$(tail -c 1 -- "$config_file" || true)" ]]; then
-      LC_ALL=C sed -i --follow-symlinks '$a'\\ "$config_file"
-  fi
+if [[ ! -f $config_file ]]; then
+    touch "$config_file"
+fi
+
+IFS="," read -r -a SERVERS <<< "$var_multiple_time_servers"
+
+valid_pattern=""
+for server in "${SERVERS[@]}"; do
+    escaped_server="${server//./\\.}"
+    if [ -z "$valid_pattern" ]; then
+        valid_pattern="${escaped_server}"
+    else
+        valid_pattern="${valid_pattern}|${escaped_server}"
+    fi
+done
+
+if [ -n "$valid_pattern" ]; then
+    sed -i -E "/^[[:space:]]*server[[:space:]]/ { /^[[:space:]]*server[[:space:]]+(${valid_pattern})([[:space:]]|$)/! s/^[[:space:]]*server/# server/ }" "$config_file"
+fi
+
+for server in "${SERVERS[@]}" ; do
+    escaped_server="${server//./\\.}"
+    if ! grep -q "^[[:space:]]*server[[:space:]]\+${escaped_server}\([[:space:]]\|$\)" "$config_file" ; then
+        printf '\nserver %s' "$server" >> "$config_file"
+    fi
+done
+if [[ -s "$config_file" ]] && [[ -n "$(tail -c 1 -- "$config_file" || true)" ]]; then
+    LC_ALL=C sed -i --follow-symlinks '$a'\\ "$config_file"
 fi
 
 else

bash remediation for rule 'xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_specify_multiple_servers' differs.
--- xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_specify_multiple_servers
+++ xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_specify_multiple_servers
@@ -8,13 +8,28 @@
 /usr/sbin/pidof ntpd || config_file="/etc/chrony.conf"
 
 if ! [ "$(grep -c '^server' "$config_file")" -gt 1 ] ; then
-  if ! grep -q '#[[:space:]]*server' "$config_file" ; then
-    for server in $(echo "$var_multiple_time_servers" | tr ',' '\n') ; do
-      printf '\nserver %s' "$server" >> "$config_file"
-    done
-  else
-    sed -i 's/#[ \t]*server/server/g' "$config_file"
+  IFS="," read -r -a SERVERS <<< "$var_multiple_time_servers"
+
+  valid_pattern=""
+  for server in "${SERVERS[@]}"; do
+      escaped_server="${server//./\\.}"
+      if [ -z "$valid_pattern" ]; then
+          valid_pattern="${escaped_server}"
+      else
+          valid_pattern="${valid_pattern}|${escaped_server}"
+      fi
+  done
+
+  if [ -n "$valid_pattern" ]; then
+      sed -i -E "/^[[:space:]]*server[[:space:]]/ { /^[[:space:]]*server[[:space:]]+(${valid_pattern})([[:space:]]|$)/! s/^[[:space:]]*server/# server/ }" "$config_file"
   fi
+
+  for server in "${SERVERS[@]}" ; do
+      escaped_server="${server//./\\.}"
+      if ! grep -q "^[[:space:]]*server[[:space:]]\+${escaped_server}\([[:space:]]\|$\)" "$config_file" ; then
+          printf '\nserver %s' "$server" >> "$config_file"
+      fi
+  done
   if [[ -s "$config_file" ]] && [[ -n "$(tail -c 1 -- "$config_file" || true)" ]]; then
       LC_ALL=C sed -i --follow-symlinks '$a'\\ "$config_file"
   fi

bash remediation for rule 'xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_specify_remote_server' differs.
--- xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_specify_remote_server
+++ xccdf_org.ssgproject.content_rule_chronyd_or_ntpd_specify_remote_server
@@ -8,13 +8,28 @@
 /usr/sbin/pidof ntpd || config_file="/etc/chrony.conf"
 
 if ! grep -q ^server "$config_file" ; then
-  if ! grep -q '#[[:space:]]*server' "$config_file" ; then
-    for server in $(echo "$var_multiple_time_servers" | tr ',' '\n') ; do
-      printf '\nserver %s' "$server" >> "$config_file"
-    done
-  else
-    sed -i 's/#[ \t]*server/server/g' "$config_file"
+  IFS="," read -r -a SERVERS <<< "$var_multiple_time_servers"
+
+  valid_pattern=""
+  for server in "${SERVERS[@]}"; do
+      escaped_server="${server//./\\.}"
+      if [ -z "$valid_pattern" ]; then
+          valid_pattern="${escaped_server}"
+      else
+          valid_pattern="${valid_pattern}|${escaped_server}"
+      fi
+  done
+
+  if [ -n "$valid_pattern" ]; then
+      sed -i -E "/^[[:space:]]*server[[:space:]]/ { /^[[:space:]]*server[[:space:]]+(${valid_pattern})([[:space:]]|$)/! s/^[[:space:]]*server/# server/ }" "$config_file"
   fi
+
+  for server in "${SERVERS[@]}" ; do
+      escaped_server="${server//./\\.}"
+      if ! grep -q "^[[:space:]]*server[[:space:]]\+${escaped_server}\([[:space:]]\|$\)" "$config_file" ; then
+          printf '\nserver %s' "$server" >> "$config_file"
+      fi
+  done
   if [[ -s "$config_file" ]] && [[ -n "$(tail -c 1 -- "$config_file" || true)" ]]; then
       LC_ALL=C sed -i --follow-symlinks '$a'\\ "$config_file"
   fi

@alanmcanonical

Copy link
Copy Markdown
Contributor Author

test result

python3 tests/automatus.py rule --libvirt qemu:///system sec-noble-amd64 --datastream build/ssg-ubuntu2404-ds.xml --remediate-using bash --profile chronyd_specify_remote_server chronyd_specify_remote_server --profile (all)
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/alan.moore@canonical.com/dev/cac/content/logs/rule-custom-2026-09-08-1024/test_suite.log
libvirt: QEMU Driver error : argument unsupported: QEMU guest agent is not configured
WARNING - Script 'multiple_sourcedir.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'correct_pool.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'sourcedir.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'confdir.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'server_not_specified.fail.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
INFO - xccdf_org.ssgproject.content_rule_chronyd_specify_remote_server
INFO - Script file_empty.fail.sh using profile (all) OK
INFO - Script correct.pass.sh using profile (all) OK
INFO - Script multiple_servers.pass.sh using profile (all) OK
INFO - Script line_missing.fail.sh using profile (all) OK
INFO - Script multiple_incorrect.fail.sh using profile (all) OK
INFO - Script file_missing.fail.sh using profile (all) OK
INFO - Script commented_sourcedir.fail.sh using profile (all) OK
INFO - Script empty_sourcedir_main.pass.sh using profile (all) OK

@jan-cerny jan-cerny added this to the 0.1.83 milestone Sep 8, 2026
@jan-cerny jan-cerny self-assigned this Sep 8, 2026
Comment thread linux_os/guide/services/ntp/chronyd_specify_remote_server/bash/shared.sh Outdated
Comment thread shared/macros/10-bash.jinja Outdated
Signed-off-by: Alan Moore <alan.moore@canonical.com>
Signed-off-by: Alan Moore <alan.moore@canonical.com>
…nterpreting backslashes

Signed-off-by: Alan Moore <alan.moore@canonical.com>
@alanmcanonical
alanmcanonical marked this pull request as draft September 8, 2026 13:02
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Sep 8, 2026
Signed-off-by: Alan Moore <alan.moore@canonical.com>
Signed-off-by: Alan Moore <alan.moore@canonical.com>
@alanmcanonical
alanmcanonical marked this pull request as ready for review September 8, 2026 14:14
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Sep 8, 2026
@alanmcanonical

Copy link
Copy Markdown
Contributor Author
  • Add escape test case
  • Comment out invalid server lines

test result

python3 tests/automatus.py rule --libvirt qemu:///system sec-noble-amd64 --datastream build/ssg-ubuntu2404-ds.xml --remediate-using bash --profile chronyd_specify_remote_server chronyd_specify_remote_server --profile (all)
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/alan.moore@canonical.com/dev/cac/content/logs/rule-custom-2026-09-08-1510/test_suite.log
libvirt: QEMU Driver error : argument unsupported: QEMU guest agent is not configured
WARNING - Script 'multiple_sourcedir.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'correct_pool.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'sourcedir.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'confdir.pass.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
WARNING - Script 'server_not_specified.fail.sh' is not applicable on 'ubuntu2404' target because its platform is 'multi_platform_fedora,multi_platform_ol,multi_platform_rhel,multi_platform_almalinux'
INFO - xccdf_org.ssgproject.content_rule_chronyd_specify_remote_server
INFO - Script file_empty.fail.sh using profile (all) OK
INFO - Script correct.pass.sh using profile (all) OK
INFO - Script multiple_servers.pass.sh using profile (all) OK
INFO - Script line_missing.fail.sh using profile (all) OK
INFO - Script multiple_incorrect.fail.sh using profile (all) OK
INFO - Script file_missing.fail.sh using profile (all) OK
INFO - Script escape_dot.fail.sh using profile (all) OK
INFO - Script commented_sourcedir.fail.sh using profile (all) OK
INFO - Script empty_sourcedir_main.pass.sh using profile (all) OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Used by openshift-ci bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants