From cdd56d62ced8ef2bd9362659f6c6291d5de7e635 Mon Sep 17 00:00:00 2001
From: HuijingHei <hhei@redhat.com>
Date: Wed, 20 Dec 2023 10:17:20 +0800
Subject: [PATCH] kargs: run `delete-if-present` and `append-if-missing` failed
 when there is existing `key`

Revert part of https://github.com/coreos/rpm-ostree/commit/1a1ef139d41d49d0907cc18d402a9ca8ba0e6de5

`ostree_kernel_args_delete_if_present()` calls
`ostree_kernel_args_contains()` which only looks at the `key`
part, which makes `delete-if-present` failed when there is
existing `key`. `append-if-missing` will hit the same problem.
```
]# rpm-ostree kargs --append=crashkernel=1G
]# rpm-ostree kargs
... crashkernel=1G
]# rpm-ostree kargs --unchanged-exit-77 --delete-if-present=crashkernel=auto
error: No karg 'crashkernel=auto' found
]# rpm-ostree kargs --unchanged-exit-77 --append-if-missing=crashkernel=auto
No changes.
```

And fix corner case when `append` and `append-if-missing` with
the same value which will append twice.

Fixes: https://github.com/coreos/rpm-ostree/issues/4718
---
 src/daemon/rpmostreed-transaction-types.cxx | 12 +++++++----
 tests/vmcheck/test-kernel-args.sh           | 22 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/daemon/rpmostreed-transaction-types.cxx b/src/daemon/rpmostreed-transaction-types.cxx
index b0c4030a7d..be4bb3e393 100644
--- a/src/daemon/rpmostreed-transaction-types.cxx
+++ b/src/daemon/rpmostreed-transaction-types.cxx
@@ -2679,6 +2679,9 @@ kernel_arg_apply_patching (KernelArgTransaction *self, RpmOstreeSysrootUpgrader
       = static_cast<char **> (vardict_lookup_strv_canonical (self->options, "append-if-missing"));
   g_autofree char **delete_if_present
       = static_cast<char **> (vardict_lookup_strv_canonical (self->options, "delete-if-present"));
+  g_autoptr (OstreeKernelArgs) existing_kargs
+      = ostree_kernel_args_from_string (self->existing_kernel_args);
+  g_auto (GStrv) existing_kargs_strv = ostree_kernel_args_to_strv (existing_kargs);
   gboolean changed = FALSE;
 
   /* Delete all the entries included in the kernel args */
@@ -2707,9 +2710,10 @@ kernel_arg_apply_patching (KernelArgTransaction *self, RpmOstreeSysrootUpgrader
   for (char **iter = append_if_missing; iter && *iter; iter++)
     {
       const char *arg = *iter;
-      if (!ostree_kernel_args_contains (kargs, arg))
+      g_auto (GStrv) kargs_strv = ostree_kernel_args_to_strv (kargs);
+      if (!g_strv_contains (kargs_strv, arg))
         {
-          ostree_kernel_args_append_if_missing (kargs, arg);
+          ostree_kernel_args_append (kargs, arg);
           changed = TRUE;
         }
     }
@@ -2717,9 +2721,9 @@ kernel_arg_apply_patching (KernelArgTransaction *self, RpmOstreeSysrootUpgrader
   for (char **iter = delete_if_present; iter && *iter; iter++)
     {
       const char *arg = *iter;
-      if (ostree_kernel_args_contains (kargs, arg))
+      if (g_strv_contains (existing_kargs_strv, arg))
         {
-          if (!ostree_kernel_args_delete_if_present (kargs, arg, error))
+          if (!ostree_kernel_args_delete (kargs, arg, error))
             return FALSE;
           changed = TRUE;
         }
diff --git a/tests/vmcheck/test-kernel-args.sh b/tests/vmcheck/test-kernel-args.sh
index d48b393bf6..e66373cfc6 100755
--- a/tests/vmcheck/test-kernel-args.sh
+++ b/tests/vmcheck/test-kernel-args.sh
@@ -178,6 +178,28 @@ vm_rpmostree kargs --delete-if-present=PACKAGE3=TEST3 --unchanged-exit-77 || rc=
 assert_streq $rc 77
 echo "ok exit 77 when unchanged kargs with unchanged-exit-77"
 
+# Test append-if-missing and delete-if-present for existing key
+vm_rpmostree kargs --append-if-missing=PACKAGE4=TEST4
+vm_rpmostree kargs > kargs.txt
+assert_file_has_content_literal kargs.txt 'PACKAGE4=TEST4'
+vm_rpmostree kargs --append-if-missing=PACKAGE4=NEWTEST
+vm_rpmostree kargs > kargs.txt
+assert_file_has_content_literal kargs.txt 'PACKAGE4=NEWTEST'
+vm_rpmostree kargs --delete-if-present=PACKAGE4=TEST --unchanged-exit-77 || rc=$?
+assert_streq $rc 77
+echo "ok for append-if-missing and delete-if-present with existing key"
+
+# Test corner case for append and append-if-missing with the same value
+vm_rpmostree kargs --append=foo --append-if-missing=foo
+vm_rpmostree kargs > kargs.txt
+assert_not_file_has_content_literal kargs.txt 'foo foo'
+assert_file_has_content_literal kargs.txt 'foo'
+vm_rpmostree kargs --append-if-missing=bar=foo --append-if-missing=bar=foo
+vm_rpmostree kargs > kargs.txt
+assert_not_file_has_content_literal kargs.txt 'bar=foo bar=foo'
+assert_file_has_content_literal kargs.txt 'bar=foo'
+echo "ok for append and append-if-missing with the same value"
+
 # Test for 'rpm-ostree kargs --editor'.
 vm_rpmostree kargs > kargs.txt
 assert_not_file_has_content_literal kargs.txt 'nonexisting'