Repository: osbuild/osbuild
Pull request #1836 (2024-08-07 10:37:26)

osbuild: make `inputs` `map()` function use fd for reply as well
We recently hit the issue that `osbuild` crashed with:
```
Unable to decode response body "Traceback (most recent call last):
  File \"/usr/bin/osbuild\", line 33, in <module>
    sys.exit(load_entry_point('osbuild==124', 'console_scripts', 'osbuild')())
  File \"/usr/lib/python3.9/site-packages/osbuild/main_cli.py\", line 181, in osbuild_cli
    r = manifest.build(
  File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 477, in build
    res = pl.run(store, monitor, libdir, debug_break, stage_timeout)
  File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 376, in run
    results = self.build_stages(store,
  File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 348, in build_stages
    r = stage.run(tree,
  File \"/usr/lib/python3.9/site-packages/osbuild/pipeline.py\", line 213, in run
    data = ipmgr.map(ip, store)
  File \"/usr/lib/python3.9/site-packages/osbuild/inputs.py\", line 94, in map
    reply, _ = client.call_with_fds(\"map\", {}, fds)
  File \"/usr/lib/python3.9/site-packages/osbuild/host.py\", line 373, in call_with_fds
    kind, data = self.protocol.decode_message(ret)
  File \"/usr/lib/python3.9/site-packages/osbuild/host.py\", line 83, in decode_message
    raise ProtocolError(\"message empty\")
osbuild.host.ProtocolError: message empty
cannot run osbuild: exit status 1" into osbuild result: invalid character 'T' looking for beginning of value
...
input/packages (org.osbuild.files): Traceback (most recent call last):
input/packages (org.osbuild.files):   File "/usr/lib/osbuild/inputs/org.osbuild.files", line 226, in <module>
input/packages (org.osbuild.files):     main()
input/packages (org.osbuild.files):   File "/usr/lib/osbuild/inputs/org.osbuild.files", line 222, in main
input/packages (org.osbuild.files):     service.main()
input/packages (org.osbuild.files):   File "/usr/lib/python3.11/site-packages/osbuild/host.py", line 250, in main
input/packages (org.osbuild.files):     self.serve()
input/packages (org.osbuild.files):   File "/usr/lib/python3.11/site-packages/osbuild/host.py", line 284, in serve
input/packages (org.osbuild.files):     self.sock.send(reply, fds=reply_fds)
input/packages (org.osbuild.files):   File "/usr/lib/python3.11/site-packages/osbuild/util/jsoncomm.py", line 407, in send
input/packages (org.osbuild.files):     n = self._socket.sendmsg([serialized], cmsg, 0)
input/packages (org.osbuild.files):         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
input/packages (org.osbuild.files): OSError: [Errno 90] Message too long
```

The underlying issue is that the reply of the `map()` call is too big for the buffer that `jsoncomm` uses. This problem existed before for the args of map and was fixed by introducing a temporary file in https://github.com/osbuild/osbuild/pull/1331 (and similarly before in https://github.com/osbuild/osbuild/pull/824).

This commit writes the return values also into a file. This should fix the crash above and make the function more symetrical as well.

Alternative/complementary version of
https://github.com/osbuild/osbuild/pull/1833

Closes: HMS-4537


Patch:
diff --git a/osbuild/inputs.py b/osbuild/inputs.py
index 229c1264..24c580e6 100644
--- a/osbuild/inputs.py
+++ b/osbuild/inputs.py
@@ -88,10 +88,12 @@ class InputManager:
             }
         }
 
-        with make_args_file(store.tmp, args) as fd:
-            fds = [fd]
+        with make_args_and_reply_files(store.tmp, args) as (fd_args, fd_reply):
+            fds = [fd_args, fd_reply]
             client = self.service_manager.start(f"input/{ip.name}", ip.info.path)
-            reply, _ = client.call_with_fds("map", {}, fds)
+            _, _ = client.call_with_fds("map", {}, fds)
+            with os.fdopen(os.dup(fd_reply)) as f:
+                reply = json.loads(f.read())
 
         path = reply["path"]
 
@@ -106,11 +108,12 @@ class InputManager:
 
 
 @contextlib.contextmanager
-def make_args_file(tmp, args):
-    with tempfile.TemporaryFile("w+", dir=tmp, encoding="utf-8") as f:
-        json.dump(args, f)
-        f.seek(0)
-        yield f.fileno()
+def make_args_and_reply_files(tmp, args):
+    with tempfile.TemporaryFile("w+", dir=tmp, encoding="utf-8") as f_args, \
+            tempfile.TemporaryFile("w+", dir=tmp, encoding="utf-8") as f_reply:
+        json.dump(args, f_args)
+        f_args.seek(0)
+        yield f_args.fileno(), f_reply.fileno()
 
 
 class InputService(host.Service):
@@ -126,9 +129,11 @@ class InputService(host.Service):
     def stop(self):
         self.unmap()
 
-    def dispatch(self, method: str, _, _fds):
+    def dispatch(self, method: str, _, fds):
         if method == "map":
-            with os.fdopen(_fds.steal(0)) as f:
+            # map() sends fd[0] to read the arguments from and fd[1] to
+            # write the reply back. This avoids running into EMSGSIZE
+            with os.fdopen(fds.steal(0)) as f:
                 args = json.load(f)
             store = StoreClient(connect_to=args["api"]["store"])
             r = self.map(store,
@@ -136,6 +141,9 @@ class InputService(host.Service):
                          args["refs"],
                          args["target"],
                          args["options"])
-            return r, None
+            with os.fdopen(fds.steal(1), "w") as f:
+                f.write(json.dumps(r))
+                f.seek(0)
+            return "{}", None
 
         raise host.ProtocolError("Unknown method")
diff --git a/osbuild/util/jsoncomm.py b/osbuild/util/jsoncomm.py
index 04821257..8a0cbb7c 100644
--- a/osbuild/util/jsoncomm.py
+++ b/osbuild/util/jsoncomm.py
@@ -35,7 +35,7 @@ class FdSet:
     def __init__(self, *, rawfds):
         for i in rawfds:
             if not isinstance(i, int) or i < 0:
-                raise ValueError()
+                raise ValueError(f"unexpected fd {i}")
 
         self._fds = rawfds
 


Test patch:
diff --git a/test/mod/test_inputs.py b/test/mod/test_inputs.py
new file mode 100644
index 00000000..915d1313
--- /dev/null
+++ b/test/mod/test_inputs.py
@@ -0,0 +1,47 @@
+import json
+import os
+from unittest.mock import patch
+
+from osbuild import inputs
+from osbuild.util.jsoncomm import FdSet
+
+
+class FakeInputService(inputs.InputService):
+    def __init__(self, args):  # pylint: disable=super-init-not-called
+        # do not call "super().__init__()" here to make it testable
+        self.map_calls = []
+
+    def map(self, _store, origin, refs, target, options):
+        self.map_calls.append([origin, refs, target, options])
+        return "complex", 2, "reply"
+
+
+def test_inputs_dispatches_map(tmp_path):
+    store_api_path = tmp_path / "api-store"
+    store_api_path.write_text("")
+
+    args_path = tmp_path / "args"
+    reply_path = tmp_path / "reply"
+    args = {
+        "api": {
+            "store": os.fspath(store_api_path),
+        },
+        "origin": "some-origin",
+        "refs": "some-refs",
+        "target": "some-target",
+        "options": "some-options",
+    }
+    args_path.write_text(json.dumps(args))
+    reply_path.write_text("")
+
+    with args_path.open() as f_args, reply_path.open("w") as f_reply:
+        fd_args, fd_reply = os.dup(f_args.fileno()), os.dup(f_reply.fileno())
+        fds = FdSet.from_list([fd_args, fd_reply])
+        fake_service = FakeInputService(args="some")
+        with patch.object(inputs, "StoreClient"):
+            r = fake_service.dispatch("map", None, fds)
+            assert r == ('{}', None)
+        assert fake_service.map_calls == [
+            ["some-origin", "some-refs", "some-target", "some-options"],
+        ]
+    assert reply_path.read_text() == '["complex", 2, "reply"]'