Fix possibly missing pg1-* options for the remote command.

Some pg1-* options are required by the remote so if they are not provided in the remote's configuration file then it may cause a configuration error, depending on the operation. This currently only applies to the pg1-path option.

This is still an issue for repo-* options but the same solution cannot be applied because some repo-* options are secure and cannot be passed on the command-line.

doc/xml/release.xml

@@ -56,6 +56,15 @@
 
                         <p>Increase buffer size for <proper>lz4</proper> compression flush.</p>
                     </release-item>
+
+                    <release-item>
+                        <release-item-contributor-list>
+                             <release-item-ideator id="andrew.lecuyer"/>
+                             <release-item-reviewer id="cynthia.shang"/>
+                        </release-item-contributor-list>
+
+                        <p>Fix possibly missing <br-option>pg1-*</br-option> options for the <cmd>remote</cmd> command.</p>
+                    </release-item>
                 </release-bug-list>
 
                 <release-feature-list>
@@ -8093,6 +8102,11 @@
             <contributor-id type="github">anarazel</contributor-id>
         </contributor>
 
+        <contributor id="andrew.lecuyer">
+            <contributor-name-display>Andrew L'Ecuyer</contributor-name-display>
+            <contributor-id type="github">andrewlecuyer</contributor-id>
+        </contributor>
+
         <contributor id="andrew.schwartz">
             <contributor-name-display>Andrew Schwartz</contributor-name-display>
             <contributor-id type="github">trinchan</contributor-id>

src/protocol/helper.c

@@ -278,6 +278,7 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int protoc
         cfgOptionSource(optConfigPath) != cfgSourceDefault ? cfgOption(optConfigPath) : NULL);
 
     // Update/remove repo/pg options that are sent to the remote
+    ConfigDefineCommand commandDefId = cfgCommandDefIdFromId(cfgCommand());
     const String *repoHostPrefix = STR(cfgDefOptionName(cfgDefOptRepoHost));
     const String *repoPrefix = strNewFmt("%s-", PROTOCOL_REMOTE_TYPE_REPO);
     const String *pgHostPrefix = STR(cfgDefOptionName(cfgDefOptPgHost));
@@ -285,7 +286,8 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int protoc
 
     for (ConfigOption optionId = 0; optionId < CFG_OPTION_TOTAL; optionId++)
     {
-        const String *optionDefName = STR(cfgDefOptionName(cfgOptionDefIdFromId(optionId)));
+        ConfigDefineOption optionDefId = cfgOptionDefIdFromId(optionId);
+        const String *optionDefName = STR(cfgDefOptionName(optionDefId));
         bool remove = false;
 
         // Remove repo host options that are not needed on the remote.  The remote is not expecting to see host settings and it
@@ -308,10 +310,10 @@ protocolRemoteParam(ProtocolStorageType protocolStorageType, unsigned int protoc
         }
         else if (strBeginsWith(optionDefName, pgPrefix))
         {
-            // Remove pg options when the remote type is repo since they won't be used
+            // Remove unrequired/defaulted pg options when the remote type is repo since they won't be used
             if (protocolStorageType == protocolStorageTypeRepo)
             {
-                remove = true;
+                remove = !cfgDefOptionRequired(commandDefId, optionDefId) || cfgDefOptionDefault(commandDefId, optionDefId) != NULL;
             }
             // Else move/remove pg options with index > 0 since they won't be used
             else if (cfgOptionIndex(optionId) > 0)

test/src/module/protocol/protocolTest.c

@@ -242,21 +242,23 @@ testRun(void)
         strLstAddZ(argList, "pgbackrest");
         strLstAddZ(argList, "--stanza=test1");
         strLstAddZ(argList, "--log-subprocess");
+        strLstAddZ(argList, "--" CFGOPT_PG1_PATH "=/unused");       // Will be passed to remote (required)
+        strLstAddZ(argList, "--" CFGOPT_PG1_PORT "=777");           // Not be passed to remote (required but has default)
         strLstAddZ(argList, "--repo1-host=repo-host");
         strLstAddZ(argList, "--repo1-host-port=444");
         strLstAddZ(argList, "--repo1-host-config=/path/pgbackrest.conf");
         strLstAddZ(argList, "--repo1-host-config-include-path=/path/include");
         strLstAddZ(argList, "--repo1-host-config-path=/path/config");
         strLstAddZ(argList, "--repo1-host-user=repo-host-user");
-        strLstAddZ(argList, "archive-get");
+        strLstAddZ(argList, CFGCMD_CHECK);
         harnessCfgLoadRaw(strLstSize(argList), strLstPtr(argList));
 
         TEST_RESULT_STR_Z(
             strLstJoin(protocolRemoteParam(protocolStorageTypeRepo, 1, 0), "|"),
             "-o|LogLevel=error|-o|Compression=no|-o|PasswordAuthentication=no|-p|444|repo-host-user@repo-host"
                 "|pgbackrest --config=/path/pgbackrest.conf --config-include-path=/path/include --config-path=/path/config"
-                " --log-level-console=off --log-level-file=info --log-level-stderr=error --log-subprocess --process=1"
-                " --remote-type=repo --stanza=test1 archive-get:remote",
+                " --log-level-console=off --log-level-file=info --log-level-stderr=error --log-subprocess --pg1-path=/unused"
+                " --process=1 --remote-type=repo --stanza=test1 check:remote",
             "remote protocol params with replacements");
 
         // -------------------------------------------------------------------------------------------------------------------------