Repository: osbuild/osbuild
Pull request #1560 (2024-01-30 15:27:06)

stages(kickstart): Add missing rootpw, initlabel, nohome
In the context of specific ostree installation we are missing some kickstart options:

1. rootpw option (despite we only need rootpw --lock, implement the full spec found here https://pykickstart.readthedocs.io/en/latest/kickstart-docs.html#rootpw)
2. initlabel a property of clearpart option
3. nohome a property of autopart 

FIXES: https://issues.redhat.com/browse/THEEDGE-3835

ldjebran: @mvo5 @achilleas-k I added a new commit with 2 rules and two negative tests , can you revisit 
ldjebran: @bcl @mvo5 Thank you for review,  updated with the style recommendation, thanks to revisit.
mvo5: > @bcl @mvo5 Thank you for review, updated with the style recommendation, thanks to revisit.

This looks good to me now! One nitpick - we care about a nice git history so it would be nice if all 4 commits could be squashed into a single one to avoid "fixup!" style commits in our main history (I hope I'm making sense?).
ldjebran: @achilleas-k @mvo5 I have made an update as one of the tests was failing on py3.6: 

```py
        # under py3.6 the message is "'' is too short" under other versions the message is "'' should be non-empty"
        ({"rootpw": {"plaintext": True, "password": ""}}, re.compile("'' should be non-empty|'' is too short")),
```

Patch:
diff --git a/stages/org.osbuild.kickstart b/stages/org.osbuild.kickstart
index 03d30cf1..2153af72 100755
--- a/stages/org.osbuild.kickstart
+++ b/stages/org.osbuild.kickstart
@@ -223,6 +223,37 @@ SCHEMA = r"""
       }
     }
   },
+  "rootpw": {
+    "type": "object",
+    "anyOf":[
+      {"required": ["lock"], "not": {"required": ["allow_ssh"]}},
+      {"required": ["iscrypted", "password"], "not": {"required": ["plaintext"]}},
+      {"required": ["plaintext", "password"], "not": {"required": ["iscrypted"]}}
+    ],
+    "properties": {
+      "lock": {
+        "type": "boolean",
+        "description": "the root account is locked by default"
+      },
+      "plaintext": {
+         "type": "boolean",
+         "description": "the password argument is assumed to be in plain text"
+      },
+      "iscrypted": {
+         "type": "boolean",
+         "description": "the password argument is assumed to already be encrypted"
+      },
+      "allow_ssh": {
+         "type": "boolean",
+         "description": "This will allow remote root logins via ssh using only the password"
+      },
+      "password": {
+         "type": "string",
+         "description": "the root password",
+         "minLength": 1
+      }
+    }
+  },
   "lang": {
     "type": "string",
     "description": "The language code (e.g. en_US.UTF-8)"
@@ -254,6 +285,10 @@ SCHEMA = r"""
         "description": "Erases all partitions from the system",
         "type": "boolean"
       },
+      "initlabel": {
+        "description": "Initializes a disk (or disks) by creating a default disk label for all disks",
+        "type": "boolean"
+      },
       "drives": {
         "description": "Specifies which drives to clear partitions from",
         "type": "array",
@@ -363,6 +398,10 @@ SCHEMA = r"""
       "pbkdf-iterations": {
         "description": "Sets the number of iterations for passphrase processing directly",
         "type": "integer"
+      },
+      "nohome": {
+        "description": "Disables automatic creation of the /home partition",
+        "type": "boolean"
       }
     }
   },
@@ -438,6 +477,19 @@ def make_users(users: Dict) -> List[str]:
     return res
 
 
+def make_rootpw(rootpw: Dict) -> str:
+    arguments = []
+    for option in ["lock", "plaintext", "iscrypted", "allow_ssh", "password"]:
+        option_value = rootpw.get(option)
+        if option_value is True:
+            arguments.append(f"--{option.replace('_', '-')}")
+        elif isinstance(option_value, str) and option_value:
+            arguments.append(option_value)
+    if arguments:
+        return f"rootpw {' '.join(arguments)}"
+    return ""
+
+
 def make_clearpart(options: Dict) -> str:
     clearpart = options.get("clearpart")
     if clearpart is None:
@@ -458,6 +510,9 @@ def make_clearpart(options: Dict) -> str:
     linux = clearpart.get("linux", False)
     if linux:
         cmd += " --linux"
+    initlabel = clearpart.get("initlabel", False)
+    if initlabel:
+        cmd += " --initlabel"
     return cmd
 
 
@@ -481,7 +536,7 @@ def make_autopart(options: Dict) -> str:
     cmd = "autopart"
     for key in ["type", "fstype", "nolvm", "encrypted", "passphrase",
                 "escrowcert", "backuppassphrase", "cipher", "luks-version",
-                "pbkdf", "pbkdf-memory", "pbkdf-time", "pbkdf-iterations"]:
+                "pbkdf", "pbkdf-memory", "pbkdf-time", "pbkdf-iterations", "nohome"]:
         if key not in autopart:
             continue
         val = autopart[key]
@@ -568,6 +623,9 @@ def main(tree, options):  # pylint: disable=too-many-branches
 
     config += make_groups(options.get("groups", {}))
     config += make_users(options.get("users", {}))
+    rootpw_command = make_rootpw(options.get("rootpw", {}))
+    if rootpw_command:
+        config += [rootpw_command]
 
     lang = options.get("lang")
     if lang:


Test patch:
diff --git a/stages/test/test_kickstart.py b/stages/test/test_kickstart.py
index d4aab3a8..03ee9ee7 100644
--- a/stages/test/test_kickstart.py
+++ b/stages/test/test_kickstart.py
@@ -1,6 +1,7 @@
 #!/usr/bin/python3
 
 import os.path
+import re
 import subprocess
 
 import pytest
@@ -20,6 +21,21 @@ TEST_INPUT = [
         },
         "lang en_US.UTF-8\nkeyboard us\ntimezone UTC",
     ),
+    ({"rootpw": {"lock": True}}, "rootpw --lock"),
+    ({"rootpw": {"plaintext": True, "password": "plaintext-password"}}, "rootpw --plaintext plaintext-password"),
+    ({"rootpw": {"iscrypted": True, "password": "encrypted-password"}}, "rootpw --iscrypted encrypted-password"),
+    (
+        {"rootpw": {"iscrypted": True, "allow_ssh": True, "password": "encrypted-password"}},
+        "rootpw --iscrypted --allow-ssh encrypted-password",
+    ),
+    (
+        {"rootpw": {"plaintext": True, "allow_ssh": True, "password": "plaintext-password"}},
+        "rootpw --plaintext --allow-ssh plaintext-password",
+    ),
+    (
+        {"rootpw": {"plaintext": True, "lock": True, "password": "plaintext-password"}},
+        "rootpw --lock --plaintext plaintext-password",
+    ),
     (
         {
             "ostree": {
@@ -59,6 +75,7 @@ TEST_INPUT = [
     ),
     ({"zerombr": True}, "zerombr"),
     ({"clearpart": {"all": True}}, "clearpart --all"),
+    ({"clearpart": {"all": True, "initlabel": True}}, "clearpart --all --initlabel"),
     (
         {"clearpart": {"drives": ["sd*|hd*|vda", "/dev/vdc"]}},
         "clearpart --drives=sd*|hd*|vda,/dev/vdc",
@@ -68,6 +85,7 @@ TEST_INPUT = [
         {"clearpart": {"drives": ["disk/by-id/scsi-58095BEC5510947BE8C0360F604351918"]}},
         "clearpart --drives=disk/by-id/scsi-58095BEC5510947BE8C0360F604351918"
     ),
+    ({"clearpart": {"drives": ["hda"], "initlabel": True}}, "clearpart --drives=hda --initlabel"),
     ({"clearpart": {"list": ["sda2", "sda3"]}}, "clearpart --list=sda2,sda3"),
     ({"clearpart": {"list": ["sda2"]}}, "clearpart --list=sda2"),
     (
@@ -121,6 +139,8 @@ TEST_INPUT = [
     ({"autopart": {"pbkdf-memory": 64}}, "autopart --pbkdf-memory=64"),
     ({"autopart": {"pbkdf-time": 128}}, "autopart --pbkdf-time=128"),
     ({"autopart": {"pbkdf-iterations": 256}}, "autopart --pbkdf-iterations=256"),
+    ({"autopart": {"nohome": True}}, "autopart --nohome"),
+    ({"autopart": {"type": "plain", "fstype": "xfs", "nohome": True}}, "autopart --type=plain --fstype=xfs --nohome"),
     ({
         "lang": "en_US.UTF-8",
         "keyboard": "us",
@@ -346,7 +366,17 @@ def test_kickstart_valid(tmp_path, stage_module, test_input, expected):  # pylin
             },
             "is not valid under any of the given schemas",
         ),
-
+        ({"rootpw": {}}, "is not valid under any of the given schemas"),
+        ({"rootpw": {"lock": True, "allow_ssh": True}}, "is not valid under any of the given schemas"),
+        ({"rootpw": {"plaintext": True}}, "is not valid under any of the given schemas"),
+        # under py3.6 the message is "'' is too short" under other versions the message is "'' should be non-empty"
+        ({"rootpw": {"plaintext": True, "password": ""}}, re.compile("'' should be non-empty|'' is too short")),
+        ({"rootpw": {"iscrypted": True}}, "is not valid under any of the given schemas"),
+        ({"rootpw": {"password": "password"}}, "is not valid under any of the given schemas"),
+        (
+            {"rootpw": {"iscrypted": True, "plaintext": True, "password": "pass"}},
+            "is not valid under any of the given schemas"
+        ),
     ],
 )
 @pytest.mark.parametrize("stage_schema", ["1"], indirect=True)