support multiple lines in config

The inih ini_parse_stream function will call the handler
multiple times when a value in spread across multiple
lines. Previously, this caused the custom handler to
override the value - implementing a last one wins method.
This has been changed to append the value instead. It now
also uses a temporary config for collecting settings from
all files and merging it with the previous one.
An integration test has also been added.

Signed-off-by: Michael Engel <[email protected]>

config/hirte/hirte.conf

@@ -9,6 +9,7 @@
 # A comma separated list of unique hirte-agent names. Only nodes with names mentioned in the list can connect to hirte
 # manager. These names are defined in the agent's configuration file under the `NodeName` option.
 # (see `hirte-agent.conf(5)`).
+# Note: The maximum line length is 500 characters. If the list exceeds this, use multiple, indented lines.
 #AllowedNodeNames=
 
 #

doc/docs/configuration.md

@@ -1,5 +1,7 @@
 # Configuration
 
+## Loading order
+
 On startup, hirte loads configuration files from the following directories:
 
 | Load order | hirte | hirte-agent |
@@ -18,3 +20,14 @@ configuration has the highest priority and all defined settings will override pr
 
 For a list of supported options incl. an explanation please refer to the
 the MAN pages for [hirte(5)](./man/hirte_conf.md) and [hirte-agent(5)](./man/hirte_agent_conf.md).
+
+## Maximum line length
+
+The maximum line length supported by hirte is 500 characters. If the characters of any key-value pair exceeds this, use
+multiple, indented lines. For example, a large number of node names in the `AllowedNodeNames` field can be split like this:
+
+```bash
+AllowedNodeNames=node1,
+  node2,
+  node3
+```

doc/man/hirte-agent.conf.5.md

@@ -11,7 +11,7 @@ The basic file definition used to bootstrap hirte-agent.
 ## Format
 
 The hirte-agent configuration file is using the
-[systemd configuration file format](https://www.freedesktop.org/software/systemd/man/systemd.syntax.html).
+[systemd configuration file format](https://www.freedesktop.org/software/systemd/man/systemd.syntax.html). Contrary to this, there is no need for the `\` symbol at the end of a line to continue a value on the next line. A value continued on multiple lines will just be concatenated by hirte. The maximum line length supported is 500 characters. If the value exceeds this limitation, use multiple, indented lines.
 
 ### **hirte-agent** section
 
@@ -90,6 +90,20 @@ LogLevel=DEBUG
 LogTarget=journald
 LogIsQuiet=false
 ```
+
+Using a value that is continued on multiple lines:
+
+```
+[hirte-agent]
+NodeName=agent-007
+ManagerAddress=tcp:
+  host=127.0.0.1,
+  port=842
+LogLevel=DEBUG
+LogTarget=journald
+LogIsQuiet=false
+```
+
 ## FILES
 
 Distributions provide the __/usr/share/hirte/config/agent.conf__ file which defines hirte-agent configuration defaults. Administrators can copy this file to __/etc/hirte/agent.conf__ and specify their own configuration.

doc/man/hirte.conf.5.md

@@ -11,7 +11,7 @@ The basic file definition used to bootstrap hirte.
 ## Format
 
 The hirte configuration file is using the
-[systemd configuration file format](https://www.freedesktop.org/software/systemd/man/systemd.syntax.html).
+[systemd configuration file format](https://www.freedesktop.org/software/systemd/man/systemd.syntax.html). Contrary to this, there is no need for the `\` symbol at the end of a line to continue a value on the next line. A value continued on multiple lines will just be concatenated by hirte. The maximum line length supported is 500 characters. If the value exceeds this limitation, use multiple, indented lines.
 
 ### **hirte** section
 
@@ -55,6 +55,8 @@ If this flag is set to `true`, no logs are written by hirte. By default the flag
 
 ## Example
 
+A basic example of a configuration file for `hirte`:
+
 ```
 [hirte]
 ManagerPort=842
@@ -64,6 +66,20 @@ LogTarget=journald
 LogIsQuiet=false
 ```
 
+Using a value that is continued on multiple lines:
+
+```
+[hirte]
+ManagerPort=842
+AllowedNodeNames=agent-007,
+   agent-123,
+   agent-456,
+   agent-789
+LogLevel=DEBUG
+LogTarget=journald
+LogIsQuiet=false
+```
+
 ## FILES
 
 Distributions provide the __/usr/share/hirte/config/hirte.conf__ file which defines hirte configuration defaults. Administrators can copy this file to __/etc/hirte/hirte.conf__ and specify their own configuration.

src/libhirte/common/cfg.c

@@ -56,10 +56,21 @@ static int parsing_handler(void *user, const char *section, const char *name, co
         if (user == NULL) {
                 return 0;
         }
+
         struct config *config = (struct config *) user;
+
+        const char *existing_value = cfg_s_get_value(config, section, name);
+        if (existing_value != NULL) {
+                _cleanup_free_ const char *new_value = strcat_dup(existing_value, value);
+                if (cfg_s_set_value(config, section, name, new_value) != 0) {
+                        return 0;
+                }
+                return 1;
+        }
         if (cfg_s_set_value(config, section, name, value) != 0) {
                 return 0;
         }
+
         return 1;
 }
 
@@ -86,6 +97,20 @@ int cfg_initialize(struct config **config) {
         return 0;
 }
 
+int cfg_copy_overwrite(struct config *src, struct config *dst) {
+        int r = 0;
+        void *item = NULL;
+        size_t i = 0;
+        while (hashmap_iter(src->cfg_store, &i, &item)) {
+                struct config_option *opt = item;
+                r = cfg_s_set_value(dst, opt->section, opt->name, opt->value);
+                if (r < 0) {
+                        return r;
+                }
+        }
+        return 0;
+}
+
 void cfg_dispose(struct config *config) {
         hashmap_free(config->cfg_store);
         free(config->default_section);
@@ -144,7 +169,22 @@ int cfg_load_from_file(struct config *config, const char *config_file) {
         if (access(config_file, R_OK) != 0) {
                 return -errno;
         }
-        return ini_parse(config_file, parsing_handler, config);
+
+        struct config *tmp_config = NULL;
+        int r = cfg_initialize(&tmp_config);
+        if (r < 0) {
+                return r;
+        }
+
+        r = ini_parse(config_file, parsing_handler, tmp_config);
+        if (r < 0) {
+                cfg_dispose(tmp_config);
+                return r;
+        }
+
+        r = cfg_copy_overwrite(tmp_config, config);
+        cfg_dispose(tmp_config);
+        return r;
 }
 
 int is_file_name_ending_with_conf(const struct dirent *entry) {

src/libhirte/common/cfg.h

@@ -64,6 +64,15 @@ struct config_option {
  */
 int cfg_initialize(struct config **config);
 
+/*
+ * Iterates over all entries in src and copies the key=value pairs
+ * into the dst config. If a key from src already exists in dst, the
+ * value is overridden.
+ *
+ * Returns 0 if successful, otherwise forwards error from cfg_s_set_value.
+ */
+int cfg_copy_overwrite(struct config *src, struct config *dst);
+
 /*
  * Dispose the application configuration object
  */

src/manager/manager.c

@@ -349,7 +349,12 @@ bool manager_parse_config(Manager *manager, const char *configfile) {
         const char *expected_nodes = cfg_get_value(manager->config, CFG_ALLOWED_NODE_NAMES);
         if (expected_nodes) {
                 char *saveptr = NULL;
-                char *name = strtok_r((char *) expected_nodes, ",", &saveptr);
+
+                /* copy string of expected nodes since */
+                _cleanup_free_ char *expected_nodes_cpy = NULL;
+                copy_str(&expected_nodes_cpy, expected_nodes);
+
+                char *name = strtok_r(expected_nodes_cpy, ",", &saveptr);
                 while (name != NULL) {
                         if (manager_find_node(manager, name) == NULL) {
                                 manager_add_node(manager, name);

tests/hirte_test/config.py

@@ -5,6 +5,10 @@
 
 class HirteConfig():
 
+    # use a smaller max line length than possible
+    # to prevent any accidental faults
+    MAX_LINE_LENGTH = 400
+
     def __init__(self, file_name: str) -> None:
         self.file_name = file_name
 
@@ -38,9 +42,13 @@ def __init__(
         self.log_is_quiet = log_is_quiet
 
     def serialize(self) -> str:
+        # use one line for each node name so the max line length
+        # supported by hirte is never exceeded
+        allowed_node_names = ",\n ".join(self.allowed_node_names)
+
         return f"""[hirte]
 ManagerPort={self.port}
-AllowedNodeNames={",".join(self.allowed_node_names)}
+AllowedNodeNames={allowed_node_names}
 LogLevel={self.log_level}
 LogTarget={self.log_target}
 LogIsQuiet={self.log_is_quiet}

tests/tests/tier0/hirte-long-multiline-config-setting/main.fmf

@@ -0,0 +1 @@
+summary: Test if hirte can properly parse a very long setting value spread across multiple lines, e.g. for the allowed node names

tests/tests/tier0/hirte-long-multiline-config-setting/test_long_multiline_config_setting.py

@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import pytest
+from typing import Dict
+
+from hirte_test.test import HirteTest
+from hirte_test.container import HirteControllerContainer, HirteNodeContainer
+from hirte_test.config import HirteControllerConfig
+
+
+def startup_verify(ctrl: HirteControllerContainer, _: Dict[str, HirteNodeContainer]):
+    result, output = ctrl.exec_run('systemctl is-active hirte')
+
+    assert result == 0
+    assert output == 'active'
+
+
+@pytest.mark.timeout(5)
+def test_long_multiline_config_setting(hirte_test: HirteTest, hirte_ctrl_default_config: HirteControllerConfig):
+    config = hirte_ctrl_default_config.deep_copy()
+    for i in range(150):
+        config.allowed_node_names.append(f"node-{i}")
+
+    hirte_test.set_hirte_controller_config(hirte_ctrl_default_config)
+
+    hirte_test.run(startup_verify)