Merge pull request #4090 from jestabro/configd-err-out

http-api: T6326: return full warning/error output through api

3 files changed, 136 insertions, 96 deletions

diff --git a/src/services/vyos-configd b/src/services/vyos-configd
index 3674d9627..2c0244a81 100755
--- a/src/services/vyos-configd
+++ b/src/services/vyos-configd
@@ -14,6 +14,8 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# pylint: disable=redefined-outer-name
+
 import os
 import sys
 import grp
@@ -23,8 +25,10 @@ import typing
 import logging
 import signal
 import importlib.util
+import io
+from contextlib import redirect_stdout
+
 import zmq
-from contextlib import contextmanager
 
 from vyos.defaults import directories
 from vyos.utils.boot import boot_configuration_complete
@@ -49,7 +53,8 @@ if debug:
 else:
     logger.setLevel(logging.INFO)
 
-SOCKET_PATH = "ipc:///run/vyos-configd.sock"
+SOCKET_PATH = 'ipc:///run/vyos-configd.sock'
+MAX_MSG_SIZE = 65535
 
 # Response error codes
 R_SUCCESS = 1
@@ -64,9 +69,6 @@ configd_env_unset_file = os.path.join(directories['data'], 'vyos-configd-env-uns
 # sourced on entering config session
 configd_env_file = '/etc/default/vyos-configd-env'
 
-session_out = None
-session_mode = None
-
 def key_name_from_file_name(f):
     return os.path.splitext(f)[0]
 
@@ -76,17 +78,19 @@ def module_name_from_key(k):
 def path_from_file_name(f):
     return os.path.join(vyos_conf_scripts_dir, f)
 
+
 # opt-in to be run by daemon
 with open(configd_include_file) as f:
     try:
         include = json.load(f)
     except OSError as e:
-        logger.critical(f"configd include file error: {e}")
+        logger.critical(f'configd include file error: {e}')
         sys.exit(1)
     except json.JSONDecodeError as e:
-        logger.critical(f"JSON load error: {e}")
+        logger.critical(f'JSON load error: {e}')
         sys.exit(1)
 
+
 # import conf_mode scripts
 (_, _, filenames) = next(iter(os.walk(vyos_conf_scripts_dir)))
 filenames.sort()
@@ -110,31 +114,17 @@ conf_mode_scripts = dict(zip(imports, modules))
 exclude_set = {key_name_from_file_name(f) for f in filenames if f not in include}
 include_set = {key_name_from_file_name(f) for f in filenames if f in include}
 
-@contextmanager
-def stdout_redirected(filename, mode):
-    saved_stdout_fd = None
-    destination_file = None
-    try:
-        sys.stdout.flush()
-        saved_stdout_fd = os.dup(sys.stdout.fileno())
-        destination_file = open(filename, mode)
-        os.dup2(destination_file.fileno(), sys.stdout.fileno())
-        yield
-    finally:
-        if saved_stdout_fd is not None:
-            os.dup2(saved_stdout_fd, sys.stdout.fileno())
-            os.close(saved_stdout_fd)
-        if destination_file is not None:
-            destination_file.close()
-
-def explicit_print(path, mode, msg):
-    try:
-        with open(path, mode) as f:
-            f.write(f"\n{msg}\n\n")
-    except OSError:
-        logger.critical("error explicit_print")
 
-def run_script(script_name, config, args) -> int:
+def write_stdout_log(file_name, msg):
+    if boot_configuration_complete():
+        return
+    with open(file_name, 'a') as f:
+        f.write(msg)
+
+
+def run_script(script_name, config, args) -> tuple[int, str]:
+    # pylint: disable=broad-exception-caught
+
     script = conf_mode_scripts[script_name]
     script.argv = args
     config.set_level([])
@@ -145,64 +135,53 @@ def run_script(script_name, config, args) -> int:
         script.apply(c)
     except ConfigError as e:
         logger.error(e)
-        explicit_print(session_out, session_mode, str(e))
-        return R_ERROR_COMMIT
+        return R_ERROR_COMMIT, str(e)
     except Exception as e:
         logger.critical(e)
-        return R_ERROR_DAEMON
+        return R_ERROR_DAEMON, str(e)
+
+    return R_SUCCESS, ''
 
-    return R_SUCCESS
 
 def initialization(socket):
-    global session_out
-    global session_mode
+    # pylint: disable=broad-exception-caught,too-many-locals
+
     # Reset config strings:
     active_string = ''
     session_string = ''
     # check first for resent init msg, in case of client timeout
     while True:
-        msg = socket.recv().decode("utf-8", "ignore")
+        msg = socket.recv().decode('utf-8', 'ignore')
         try:
             message = json.loads(msg)
-            if message["type"] == "init":
-                resp = "init"
+            if message['type'] == 'init':
+                resp = 'init'
                 socket.send(resp.encode())
-        except:
+        except Exception:
             break
 
     # zmq synchronous for ipc from single client:
     active_string = msg
-    resp = "active"
+    resp = 'active'
     socket.send(resp.encode())
-    session_string = socket.recv().decode("utf-8", "ignore")
-    resp = "session"
+    session_string = socket.recv().decode('utf-8', 'ignore')
+    resp = 'session'
     socket.send(resp.encode())
-    pid_string = socket.recv().decode("utf-8", "ignore")
-    resp = "pid"
+    pid_string = socket.recv().decode('utf-8', 'ignore')
+    resp = 'pid'
     socket.send(resp.encode())
-    sudo_user_string = socket.recv().decode("utf-8", "ignore")
-    resp = "sudo_user"
+    sudo_user_string = socket.recv().decode('utf-8', 'ignore')
+    resp = 'sudo_user'
     socket.send(resp.encode())
-    temp_config_dir_string = socket.recv().decode("utf-8", "ignore")
-    resp = "temp_config_dir"
+    temp_config_dir_string = socket.recv().decode('utf-8', 'ignore')
+    resp = 'temp_config_dir'
     socket.send(resp.encode())
-    changes_only_dir_string = socket.recv().decode("utf-8", "ignore")
-    resp = "changes_only_dir"
+    changes_only_dir_string = socket.recv().decode('utf-8', 'ignore')
+    resp = 'changes_only_dir'
     socket.send(resp.encode())
 
-    logger.debug(f"config session pid is {pid_string}")
-    logger.debug(f"config session sudo_user is {sudo_user_string}")
-
-    try:
-        session_out = os.readlink(f"/proc/{pid_string}/fd/1")
-        session_mode = 'w'
-    except FileNotFoundError:
-        session_out = None
-
-    # if not a 'live' session, for example on boot, write to file
-    if not session_out or not boot_configuration_complete():
-        session_out = script_stdout_log
-        session_mode = 'a'
+    logger.debug(f'config session pid is {pid_string}')
+    logger.debug(f'config session sudo_user is {sudo_user_string}')
 
     os.environ['SUDO_USER'] = sudo_user_string
     if temp_config_dir_string:
@@ -229,10 +208,12 @@ def initialization(socket):
 
     return config
 
-def process_node_data(config, data, last: bool = False) -> int:
+
+def process_node_data(config, data, _last: bool = False) -> tuple[int, str]:
     if not config:
-        logger.critical(f"Empty config")
-        return R_ERROR_DAEMON
+        out = 'Empty config'
+        logger.critical(out)
+        return R_ERROR_DAEMON, out
 
     script_name = None
     os.environ['VYOS_TAGNODE_VALUE'] = ''
@@ -246,8 +227,9 @@ def process_node_data(config, data, last: bool = False) -> int:
     if res.group(2):
         script_name = res.group(2)
     if not script_name:
-        logger.critical(f"Missing script_name")
-        return R_ERROR_DAEMON
+        out = 'Missing script_name'
+        logger.critical(out)
+        return R_ERROR_DAEMON, out
     if res.group(3):
         args = res.group(3).split()
     args.insert(0, f'{script_name}.py')
@@ -259,26 +241,55 @@ def process_node_data(config, data, last: bool = False) -> int:
     scripts_called.append(script_record)
 
     if script_name not in include_set:
-        return R_PASS
+        return R_PASS, ''
+
+    with redirect_stdout(io.StringIO()) as o:
+        result, err_out = run_script(script_name, config, args)
+    amb_out = o.getvalue()
+    o.close()
+
+    out = amb_out + err_out
+
+    return result, out
+
 
-    with stdout_redirected(session_out, session_mode):
-        result = run_script(script_name, config, args)
+def send_result(sock, err, msg):
+    msg_size = min(MAX_MSG_SIZE, len(msg)) if msg else 0
+
+    err_rep = err.to_bytes(1, byteorder=sys.byteorder)
+    logger.debug(f'Sending reply: {err}')
+    sock.send(err_rep)
+
+    # size req from vyshim client
+    size_req = sock.recv().decode()
+    logger.debug(f'Received request: {size_req}')
+    msg_size_rep = hex(msg_size).encode()
+    sock.send(msg_size_rep)
+    logger.debug(f'Sending reply: {msg_size}')
+
+    if msg_size > 0:
+        # send req is sent from vyshim client only if msg_size > 0
+        send_req = sock.recv().decode()
+        logger.debug(f'Received request: {send_req}')
+        sock.send(msg.encode())
+        logger.debug('Sending reply with output')
+
+    write_stdout_log(script_stdout_log, msg)
 
-    return result
 
 def remove_if_file(f: str):
     try:
         os.remove(f)
     except FileNotFoundError:
         pass
-    except OSError:
-        raise
+
 
 def shutdown():
     remove_if_file(configd_env_file)
     os.symlink(configd_env_unset_file, configd_env_file)
     sys.exit(0)
 
+
 if __name__ == '__main__':
     context = zmq.Context()
     socket = context.socket(zmq.REP)
@@ -294,6 +305,7 @@ if __name__ == '__main__':
     os.environ['VYOS_CONFIGD'] = 't'
 
     def sig_handler(signum, frame):
+        # pylint: disable=unused-argument
         shutdown()
 
     signal.signal(signal.SIGTERM, sig_handler)
@@ -308,20 +320,19 @@ if __name__ == '__main__':
     while True:
         #  Wait for next request from client
         msg = socket.recv().decode()
-        logger.debug(f"Received message: {msg}")
+        logger.debug(f'Received message: {msg}')
         message = json.loads(msg)
 
-        if message["type"] == "init":
-            resp = "init"
+        if message['type'] == 'init':
+            resp = 'init'
             socket.send(resp.encode())
             config = initialization(socket)
-        elif message["type"] == "node":
-            res = process_node_data(config, message["data"], message["last"])
-            response = res.to_bytes(1, byteorder=sys.byteorder)
-            logger.debug(f"Sending response {res}")
-            socket.send(response)
-            if message["last"] and config:
+        elif message['type'] == 'node':
+            res, out = process_node_data(config, message['data'], message['last'])
+            send_result(socket, res, out)
+
+            if message['last'] and config:
                 scripts_called = getattr(config, 'scripts_called', [])
                 logger.debug(f'scripts_called: {scripts_called}')
         else:
-            logger.critical(f"Unexpected message: {message}")
+            logger.critical(f'Unexpected message: {message}')
diff --git a/src/services/vyos-http-api-server b/src/services/vyos-http-api-server
index 97633577d..91100410c 100755
--- a/src/services/vyos-http-api-server
+++ b/src/services/vyos-http-api-server
@@ -577,7 +577,9 @@ def _configure_op(data: Union[ConfigureModel, ConfigureListModel,
             background_tasks.add_task(call_commit, session)
             msg = self_ref_msg
         else:
-            session.commit()
+            # capture non-fatal warnings
+            out = session.commit()
+            msg = out if out else msg
 
         logger.info(f"Configuration modified via HTTP API using key '{app.state.vyos_id}'")
     except ConfigSessionError as e:
diff --git a/src/shim/vyshim.c b/src/shim/vyshim.c
index a78f62a7b..68e6c4015 100644
--- a/src/shim/vyshim.c
+++ b/src/shim/vyshim.c
@@ -67,6 +67,8 @@ void timer_handler(int);
 
 double get_posix_clock_time(void);
 
+static char * s_recv_string (void *, int);
+
 int main(int argc, char* argv[])
 {
     // string for node data: conf_mode script and tagnode, if applicable
@@ -119,31 +121,44 @@ int main(int argc, char* argv[])
     zmq_recv(requester, error_code, 1, 0);
     debug_print("Received node data receipt\n");
 
-    int err = (int)error_code[0];
+    char msg_size_str[7];
+    zmq_send(requester, "msg_size", 8, 0);
+    zmq_recv(requester, msg_size_str, 6, 0);
+    msg_size_str[6] = '\0';
+    int msg_size = (int)strtol(msg_size_str, NULL, 16);
+    debug_print("msg_size: %d\n", msg_size);
+
+    if (msg_size > 0) {
+        zmq_send(requester, "send", 4, 0);
+        char *msg = s_recv_string(requester, msg_size);
+        printf("%s", msg);
+        free(msg);
+    }
 
     free(string_node_data_msg);
 
-    zmq_close(requester);
-    zmq_ctx_destroy(context);
+    int err = (int)error_code[0];
+    int ret = 0;
 
     if (err & PASS) {
         debug_print("Received PASS\n");
-        int ret = pass_through(argv, ex_index);
-        return ret;
+        ret = pass_through(argv, ex_index);
     }
 
     if (err & ERROR_DAEMON) {
         debug_print("Received ERROR_DAEMON\n");
-        int ret = pass_through(argv, ex_index);
-        return ret;
+        ret = pass_through(argv, ex_index);
     }
 
     if (err & ERROR_COMMIT) {
         debug_print("Received ERROR_COMMIT\n");
-        return -1;
+        ret = -1;
     }
 
-    return 0;
+    zmq_close(requester);
+    zmq_ctx_destroy(context);
+
+    return ret;
 }
 
 int initialization(void* Requester)
@@ -342,3 +357,15 @@ double get_posix_clock_time(void)
 double get_posix_clock_time(void)
 {return (double)0;}
 #endif
+
+//  Receive string from socket and convert into C string
+static char * s_recv_string (void *socket, int bufsize) {
+    char * buffer = (char *)malloc(bufsize+1);
+    int size = zmq_recv(socket, buffer, bufsize, 0);
+    if (size == -1)
+        return NULL;
+    if (size > bufsize)
+        size = bufsize;
+    buffer[size] = '\0';
+    return buffer;
+}