From d38665a2af5c541db771a44625fa5070fcb952f3 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Mon, 26 Feb 2024 09:29:48 +0100
Subject: [PATCH] util: tweak bls.options_append() support no/multiple options

The BLS specification [0] says the `options` field is optional and
can also appear multiple times. This commit tweaks the code to
deal with these corner cases and also adds tests that ensure that
this works correctly.

It also tweaks the file handling to be atomic.

[0] https://uapi-group.org/specifications/specs/boot_loader_specification/
---
 osbuild/util/bls.py       | 11 +++++---
 test/mod/test_util_bls.py | 56 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 test/mod/test_util_bls.py

diff --git a/osbuild/util/bls.py b/osbuild/util/bls.py
index c4922bec..a0989593 100644
--- a/osbuild/util/bls.py
+++ b/osbuild/util/bls.py
@@ -3,6 +3,7 @@ Function for appending parameters to
 Boot Loader Specification (BLS).
 """
 import glob
+import os
 from typing import List
 
 
@@ -23,12 +24,16 @@ def options_append(root_path: str, kernel_arguments: List[str]) -> None:
     if len(bls_conf_files) == 0:
         raise RuntimeError(f"no BLS configuration found in {bls_glob}")
     for entry in bls_conf_files:
-        # Read in the file and then append to the options line.
         with open(entry, encoding="utf8") as f:
             lines = f.read().splitlines()
-        with open(entry, "w", encoding="utf8") as f:
+        with open(entry + ".tmp", "w", encoding="utf8") as f:
+            found_opts_line = False
             for line in lines:
-                if line.startswith('options '):
+                if not found_opts_line and line.startswith('options '):
                     f.write(f"{line} {' '.join(kernel_arguments)}\n")
+                    found_opts_line = True
                 else:
                     f.write(f"{line}\n")
+            if not found_opts_line:
+                f.write(f"options {' '.join(kernel_arguments)}\n")
+        os.rename(entry + ".tmp", entry)
diff --git a/test/mod/test_util_bls.py b/test/mod/test_util_bls.py
new file mode 100644
index 00000000..57457b90
--- /dev/null
+++ b/test/mod/test_util_bls.py
@@ -0,0 +1,56 @@
+#
+# Test for the util/bls.py
+#
+
+import textwrap
+
+from osbuild.util import bls
+
+
+def make_fake_bls_file(rootdir, kernel_opts):
+    if kernel_opts:
+        options = f"options {kernel_opts}"
+    else:
+        options = ""
+    bls_path = rootdir / "loader/entries/test.conf"
+    bls_path.parent.mkdir(exist_ok=True, parents=True)
+    bls_path.write_text(textwrap.dedent(f"""\
+    title        Fedora 19 (Rawhide)
+    sort-key     fedora
+    machine-id   6a9857a393724b7a981ebb5b8495b9ea
+    version      3.8.0-2.fc19.x86_64
+    {options}
+    architecture x64
+    linux        /6a9857a393724b7a981ebb5b8495b9ea/3.8.0-2.fc19.x86_64/linux
+    initrd       /6a9857a393724b7a981ebb5b8495b9ea/3.8.0-2.fc19.x86_64/initrd
+    """), encoding="utf8")
+    return bls_path
+
+
+def test_options_append_one(tmp_path):
+    bls_path = make_fake_bls_file(tmp_path, "root=/dev/sda1 quiet")
+    bls.options_append(tmp_path, ["splash", "console=ttyS0"])
+    new_content = bls_path.read_text(encoding="utf8")
+    assert "\noptions root=/dev/sda1 quiet splash console=ttyS0\n" in new_content
+
+
+def test_options_append_none(tmp_path):
+    bls_path = make_fake_bls_file(tmp_path, "")
+    bls.options_append(tmp_path, ["splash", "console=ttyS0"])
+    new_content = bls_path.read_text(encoding="utf8")
+    assert "\noptions splash console=ttyS0\n" in new_content
+
+
+def test_options_append_multiple(tmp_path):
+    bls_path = make_fake_bls_file(tmp_path, "root=/dev/sda1")
+    with bls_path.open("a") as fp:
+        fp.write("options quiet")
+    bls.options_append(tmp_path, ["splash", "console=ttyS0"])
+    new_content = bls_path.read_text(encoding="utf8")
+    options = [line for line in new_content.split("\n")
+               if line.startswith("options ")]
+    # note that the new options only got added once
+    assert options == [
+        "options root=/dev/sda1 splash console=ttyS0",
+        "options quiet",
+    ]