From 9a18e43af7a1e99ee89a115c5c6371249a5ef9b6 Mon Sep 17 00:00:00 2001
From: Jelle van der Waa <jvanderwaa@redhat.com>
Date: Wed, 4 Oct 2023 17:49:09 +0200
Subject: [PATCH] kdump: always set kdump.conf defaults

When an administrator clears core_collector setting and we save a new
configuration the core_collector value is set to `makedumpfile` without
arguments. This is an invalid setting for kdump and not what the
kdump.conf man page recommends as default setting.

The downside of hardcoding the default recommendation is that it may
change over time, so this change includes a test to validate that when
absent we re-set the default value.
---
 pkg/kdump/config-client.js      |  9 ++++++++-
 pkg/kdump/test-config-client.js |  1 +
 test/verify/check-kdump         | 16 ++++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/pkg/kdump/config-client.js b/pkg/kdump/config-client.js
index 0cff9fe5508c..238cda9db9c8 100644
--- a/pkg/kdump/config-client.js
+++ b/pkg/kdump/config-client.js
@@ -24,6 +24,8 @@ const knownKeys = [
     "raw", "nfs", "ssh", "sshkey", "path", "core_collector", "kdump_post", "kdump_pre", "extra_bins", "extra_modules",
     "default", "force_rebuild", "override_resettable", "dracut_args", "fence_kdump_args", "fence_kdump_nodes"
 ];
+// man kdump.conf suggests this as default configuration
+const defaultCoreCollector = "makedumpfile -l --message-level 7 -d 31";
 
 /* Parse an ini-style config file
  * and monitor it for changes
@@ -264,6 +266,11 @@ export class ConfigFile {
                                 .split(" ")
                                 .filter(e => e != "-F")
                                 .join(" ");
+            } else {
+                settings._internal.core_collector = { value: defaultCoreCollector };
+                if (target.type === "ssh") {
+                    settings._internal.core_collector.value += " -F";
+                }
             }
         }
         // compression
@@ -273,7 +280,7 @@ export class ConfigFile {
                 if ("core_collector" in settings._internal)
                     settings._internal.core_collector.value = settings._internal.core_collector.value + " -c";
                 else
-                    settings._internal.core_collector = { value: "makedumpfile -c" };
+                    settings._internal.core_collector = { value: defaultCoreCollector };
             } else {
                 // disable compression
                 if ("core_collector" in this.settings._internal) {
diff --git a/pkg/kdump/test-config-client.js b/pkg/kdump/test-config-client.js
index 6eb84592d26b..7ff9a1878656 100644
--- a/pkg/kdump/test-config-client.js
+++ b/pkg/kdump/test-config-client.js
@@ -40,6 +40,7 @@ const changedConfig = [
     "",
     "#key value #comment",
     "hooray value",
+    "core_collector makedumpfile -l --message-level 7 -d 31",
     ""
 ].join("\n");
 
diff --git a/test/verify/check-kdump b/test/verify/check-kdump
index eeed8e9f9379..70c0d08c2cee 100755
--- a/test/verify/check-kdump
+++ b/test/verify/check-kdump
@@ -195,6 +195,22 @@ class TestKdump(KdumpHelpers):
         self.login_and_go("/kdump")
         b.wait_visible("#app")
 
+        # Check defaults
+        current = m.execute("grep '^core_collector' /etc/kdump.conf").strip()
+        m.execute("sed -i /^core_collector/d /etc/kdump.conf")
+        # Drop the custom path so we can make sure our changes are synced to JavaScript
+        m.execute("sed -i 's#^path /var/crash#path /var/tmp#' /etc/kdump.conf")
+        last_modified = m.execute("stat /etc/kdump.conf")
+        b.wait_text("#kdump-change-target", "locally in /var/tmp")
+        b.click("#kdump-change-target")
+        b.wait_visible("#kdump-settings-dialog")
+        b.set_input_text("#kdump-settings-local-directory", "/var/crash")
+        b.click("button:contains('Save')")
+        b.wait_not_present("#kdump-settings-dialog")
+        new = m.execute("grep '^core_collector' /etc/kdump.conf").strip()
+        self.assertEqual(current, new)
+        self.assertNotEqual(last_modified, m.execute("stat /etc/kdump.conf"))
+
         # Check remote ssh location
         b.click("#kdump-change-target")
         b.wait_visible("#kdump-settings-dialog")