Factor remote process exec out of protocolRemoteGet().

This allows protocolRemoteExec() to be shimmed, which means the remote can be run as a child of the test process, simplifying coverage testing.

The shim does not need SSH parameters, so also split those out into a separate function and update the tests to match.

doc/xml/release.xml

@@ -211,6 +211,8 @@
                         <commit subject="Fix shims with more than one function."/>
                         <commit subject="Protocol shim improvements."/>
                         <commit subject="Add local process shim to archive-get/archive-push unit tests."/>
+                        <commit subject="Set buffer-size in the configuration test harness."/>
+                        <commit subject="Factor remote process exec out of protocolRemoteGet()."/>
 
                         <p>Add local processs shim.</p>
                     </release-item>

src/protocol/helper.c

@@ -316,31 +316,6 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int hostId
     // Is this a repo remote?
     bool isRepo = protocolStorageType == protocolStorageTypeRepo;
 
-    // Fixed parameters for ssh command
-    StringList *result = strLstNew();
-    strLstAddZ(result, "-o");
-    strLstAddZ(result, "LogLevel=error");
-    strLstAddZ(result, "-o");
-    strLstAddZ(result, "Compression=no");
-    strLstAddZ(result, "-o");
-    strLstAddZ(result, "PasswordAuthentication=no");
-
-    // Append port if specified
-    ConfigOption optHostPort = isRepo ? cfgOptRepoHostPort : cfgOptPgHostPort;
-
-    if (cfgOptionIdxTest(optHostPort, hostIdx))
-    {
-        strLstAddZ(result, "-p");
-        strLstAdd(result, strNewFmt("%u", cfgOptionIdxUInt(optHostPort, hostIdx)));
-    }
-
-    // Append user/host
-    strLstAdd(
-        result,
-        strNewFmt(
-            "%s@%s", strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHostUser : cfgOptPgHostUser, hostIdx)),
-            strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHost : cfgOptPgHost, hostIdx))));
-
     // Option replacements
     KeyValue *optionReplace = kvNew();
 
@@ -444,14 +419,99 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int hostId
     // Add the remote type
     kvPut(optionReplace, VARSTRDEF(CFGOPT_REMOTE_TYPE), VARSTR(strIdToStr(protocolStorageType)));
 
-    StringList *commandExec = cfgExecParam(cfgCommand(), cfgCmdRoleRemote, optionReplace, false, true);
+    FUNCTION_LOG_RETURN(STRING_LIST, cfgExecParam(cfgCommand(), cfgCmdRoleRemote, optionReplace, false, true));
-    strLstInsert(commandExec, 0, cfgOptionIdxStr(isRepo ? cfgOptRepoHostCmd : cfgOptPgHostCmd, hostIdx));
+}
-    strLstAdd(result, strLstJoin(commandExec, " "));
+
+// Helper to add SSH parameters when executing the remote via SSH
+static StringList *
+protocolRemoteParamSsh(const ProtocolStorageType protocolStorageType, const unsigned int hostIdx)
+{
+    FUNCTION_LOG_BEGIN(logLevelDebug);
+        FUNCTION_LOG_PARAM(STRING_ID, protocolStorageType);
+        FUNCTION_LOG_PARAM(UINT, hostIdx);
+    FUNCTION_LOG_END();
+
+    StringList *result = NULL;
+
+    MEM_CONTEXT_TEMP_BEGIN()
+    {
+        // Is this a repo remote?
+        bool isRepo = protocolStorageType == protocolStorageTypeRepo;
+
+        // Fixed parameters for ssh command
+        result = strLstNew();
+        strLstAddZ(result, "-o");
+        strLstAddZ(result, "LogLevel=error");
+        strLstAddZ(result, "-o");
+        strLstAddZ(result, "Compression=no");
+        strLstAddZ(result, "-o");
+        strLstAddZ(result, "PasswordAuthentication=no");
+
+        // Append port if specified
+        ConfigOption optHostPort = isRepo ? cfgOptRepoHostPort : cfgOptPgHostPort;
+
+        if (cfgOptionIdxTest(optHostPort, hostIdx))
+        {
+            strLstAddZ(result, "-p");
+            strLstAdd(result, strNewFmt("%u", cfgOptionIdxUInt(optHostPort, hostIdx)));
+        }
+
+        // Append user/host
+        strLstAdd(
+            result,
+            strNewFmt(
+                "%s@%s", strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHostUser : cfgOptPgHostUser, hostIdx)),
+                strZ(cfgOptionIdxStr(isRepo ? cfgOptRepoHost : cfgOptPgHost, hostIdx))));
+
+        // Add remote command and parameters
+        StringList *paramList = protocolRemoteParam(protocolStorageType, hostIdx);
+
+        strLstInsert(paramList, 0, cfgOptionIdxStr(isRepo ? cfgOptRepoHostCmd : cfgOptPgHostCmd, hostIdx));
+        strLstAdd(result, strLstJoin(paramList, " "));
+
+        // Move to prior context
+        strLstMove(result, memContextPrior());
+    }
+    MEM_CONTEXT_TEMP_END();
 
     FUNCTION_LOG_RETURN(STRING_LIST, result);
 }
 
 /**********************************************************************************************************************************/
+// Helper to execute the local process. This is a separate function solely so that it can be shimmed during testing.
+static void
+protocolRemoteExec(
+    ProtocolHelperClient *const helper, const ProtocolStorageType protocolStorageType, const unsigned int hostIdx,
+    const unsigned int processId)
+{
+    FUNCTION_TEST_BEGIN();
+        FUNCTION_TEST_PARAM_P(VOID, helper);
+        FUNCTION_TEST_PARAM(ENUM, protocolStorageType);
+        FUNCTION_TEST_PARAM(UINT, hostIdx);
+        FUNCTION_TEST_PARAM(UINT, processId);
+    FUNCTION_TEST_END();
+
+    ASSERT(helper != NULL);
+
+    // Execute the protocol command
+    const char *const host =
+        strZ(cfgOptionIdxStr(protocolStorageType == protocolStorageTypeRepo ? cfgOptRepoHost : cfgOptPgHost, hostIdx));
+
+    helper->exec = execNew(
+        cfgOptionStr(cfgOptCmdSsh), protocolRemoteParamSsh(protocolStorageType, hostIdx),
+        strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u process on '%s'", processId, host), cfgOptionUInt64(cfgOptProtocolTimeout));
+    execOpen(helper->exec);
+
+    // Create protocol object
+    helper->client = protocolClientNew(
+        strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u protocol on '%s'", processId, host), PROTOCOL_SERVICE_REMOTE_STR,
+        execIoRead(helper->exec), execIoWrite(helper->exec));
+
+    protocolClientMove(helper->client, execMemContext(helper->exec));
+
+    FUNCTION_TEST_RETURN_VOID();
+}
+
 ProtocolClient *
 protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostIdx)
 {
@@ -496,19 +556,7 @@ protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostIdx)
     {
         MEM_CONTEXT_BEGIN(protocolHelper.memContext)
         {
-            unsigned int optHost = isRepo ? cfgOptRepoHost : cfgOptPgHost;
+            protocolRemoteExec(protocolHelperClient, protocolStorageType, hostIdx, processId);
-
-            // Execute the protocol command
-            protocolHelperClient->exec = execNew(
-                cfgOptionStr(cfgOptCmdSsh), protocolRemoteParam(protocolStorageType, hostIdx),
-                strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u process on '%s'", processId, strZ(cfgOptionIdxStr(optHost, hostIdx))),
-                cfgOptionUInt64(cfgOptProtocolTimeout));
-            execOpen(protocolHelperClient->exec);
-
-            // Create protocol object
-            protocolHelperClient->client = protocolClientNew(
-                strNewFmt(PROTOCOL_SERVICE_REMOTE "-%u protocol on '%s'", processId, strZ(cfgOptionIdxStr(optHost, hostIdx))),
-                PROTOCOL_SERVICE_REMOTE_STR, execIoRead(protocolHelperClient->exec), execIoWrite(protocolHelperClient->exec));
 
             // Get cipher options from the remote if none are locally configured
             if (isRepo && cfgOptionIdxStrId(cfgOptRepoCipherType, hostIdx) == cipherTypeNone)
@@ -526,8 +574,6 @@ protocolRemoteGet(ProtocolStorageType protocolStorageType, unsigned int hostIdx)
                     cfgOptionIdxSet(cfgOptRepoCipherPass, hostIdx, cfgSourceConfig, varLstGet(optionList, 1));
                 }
             }
-
-            protocolClientMove(protocolHelperClient->client, execMemContext(protocolHelperClient->exec));
         }
         MEM_CONTEXT_END();
     }

test/src/module/protocol/protocolTest.c

@@ -285,7 +285,7 @@ testRun(void)
     }
 
     // *****************************************************************************************************************************
-    if (testBegin("protocolRemoteParam()"))
+    if (testBegin("protocolRemoteParam() and protocolRemoteParamSsh()"))
     {
         storagePutP(storageNewWriteP(storageTest, STRDEF("pgbackrest.conf")), bufNew(0));
 
@@ -303,7 +303,7 @@ testRun(void)
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STRLST_Z(
-            protocolRemoteParam(protocolStorageTypeRepo, 0),
+            protocolRemoteParamSsh(protocolStorageTypeRepo, 0),
             "-o\nLogLevel=error\n-o\nCompression=no\n-o\nPasswordAuthentication=no\nrepo-host-user@repo-host\n"
                 "pgbackrest --exec-id=1-test --log-level-console=off --log-level-file=off --log-level-stderr=error"
                 " --pg1-path=/path/to/pg --process=0 --remote-type=repo --repo=1 --stanza=test1 archive-get:remote\n",
@@ -326,7 +326,7 @@ testRun(void)
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STRLST_Z(
-            protocolRemoteParam(protocolStorageTypeRepo, 0),
+            protocolRemoteParamSsh(protocolStorageTypeRepo, 0),
             "-o\nLogLevel=error\n-o\nCompression=no\n-o\nPasswordAuthentication=no\n-p\n444\nrepo-host-user@repo-host\n"
                 "pgbackrest --config=/path/pgbackrest.conf --config-include-path=/path/include --config-path=/path/config"
                 " --exec-id=1-test --log-level-console=off --log-level-file=info --log-level-stderr=error --log-subprocess"
@@ -346,7 +346,7 @@ testRun(void)
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STRLST_Z(
-            protocolRemoteParam(protocolStorageTypeRepo, 0),
+            protocolRemoteParamSsh(protocolStorageTypeRepo, 0),
             "-o\nLogLevel=error\n-o\nCompression=no\n-o\nPasswordAuthentication=no\npgbackrest@repo-host\n"
                 "pgbackrest --exec-id=1-test --log-level-console=off --log-level-file=off --log-level-stderr=error"
                 " --pg1-path=/path/to/pg --process=3 --remote-type=repo --repo=1 --stanza=test1 archive-get:remote\n",
@@ -363,7 +363,7 @@ testRun(void)
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STRLST_Z(
-            protocolRemoteParam(protocolStorageTypePg, 0),
+            protocolRemoteParamSsh(protocolStorageTypePg, 0),
             "-o\nLogLevel=error\n-o\nCompression=no\n-o\nPasswordAuthentication=no\npostgres@pg1-host\n"
                 "pgbackrest --exec-id=1-test --log-level-console=off --log-level-file=off --log-level-stderr=error"
                 " --pg1-path=/path/to/1 --process=0 --remote-type=pg --stanza=test1 backup:remote\n",
@@ -385,7 +385,7 @@ testRun(void)
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STRLST_Z(
-            protocolRemoteParam(protocolStorageTypePg, 1),
+            protocolRemoteParamSsh(protocolStorageTypePg, 1),
             "-o\nLogLevel=error\n-o\nCompression=no\n-o\nPasswordAuthentication=no\npostgres@pg2-host\n"
                 "pgbackrest --exec-id=1-test --log-level-console=off --log-level-file=off --log-level-stderr=error"
                 " --pg1-path=/path/to/2 --process=4 --remote-type=pg --stanza=test1 backup:remote\n",
@@ -407,7 +407,7 @@ testRun(void)
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STRLST_Z(
-            protocolRemoteParam(protocolStorageTypePg, 1),
+            protocolRemoteParamSsh(protocolStorageTypePg, 1),
             "-o\nLogLevel=error\n-o\nCompression=no\n-o\nPasswordAuthentication=no\npostgres@pg3-host\n"
                 "pgbackrest --exec-id=1-test --log-level-console=off --log-level-file=off --log-level-stderr=error"
                 " --pg1-path=/path/to/3 --pg1-port=3333 --pg1-socket-path=/socket3 --process=4 --remote-type=pg --stanza=test1"