SSH hook: deprecated the use of the SERVER_PORT env var in hook con…

…fig file

CHANGELOG.md

@@ -15,11 +15,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - MPI hook: verbosity levels for log messages about ABI compatibility and library replacements have been slightly adjusted.
   In particular, a warning about adding libraries into the container has been moved to a higher verbosity level
   (i.e. it will only be displayed when using the `--verbose` or `--debug` global command-line options).
+- SSH hook: the default port used by the Dropbear server is now set through the `SERVER_PORT_DEFAULT` environment variable in the hook JSON configuration file.
+  The `SERVER_PORT` variable is still supported for backward compatibility reasons, although `SERVER_PORT_DEFAULT` takes precedence if set.
+
+### Deprecated
+
+- SSH hook: usage of the `SERVER_PORT` environment variable in the hook JSON configuration file has been deprecated.
+  Support for it will be removed in a future release.
 
 ### Fixed
 
 - Glibc hook: fixed detection of the container's glibc version, which was causing a shell-init error on some systems
-- SSH hook: the `SERVER_PORT` environment variable in the JSON configuration file has been renamed to `SERVER_PORT_DEFAULT`
 
 
 ## [1.6.3]

doc/config/ssh-hook.rst

@@ -43,6 +43,10 @@ environment variables must be defined:
   daemon that could be running on the host. This value can be overridden by setting the
   ``com.hooks.ssh.port`` annotation for the container.
 
+  ``SERVER_PORT_DEFAULT`` takes precedence over the deprecated ``SERVER_PORT``
+  environment variable, which serves the same purpose.
+  Support for ``SERVER_PORT`` will be removed in a future release.
+
 The following optional environment variables can also be defined:
 
 * ``OVERLAY_MOUNT_HOME_SSH``: When set to ``False`` (case-insensitive), an overlay

src/hooks/ssh/SshHook.cpp

@@ -81,7 +81,17 @@ void SshHook::startSshDaemon() {
 
     dropbearRelativeDirInContainer = boost::filesystem::path("/opt/oci-hooks/ssh/dropbear");
     dropbearDirInHost = sarus::common::getEnvironmentVariable("DROPBEAR_DIR");
-    serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT_DEFAULT"));
+    try {
+        serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT"));
+    } catch (sarus::common::Error&) {}
+    try {
+        serverPort = std::stoi(sarus::common::getEnvironmentVariable("SERVER_PORT_DEFAULT"));
+    } catch (sarus::common::Error& e) {
+        if (serverPort == 0) {
+            SARUS_RETHROW_ERROR(e, "At least one of the environment variables SERVER_PORT_DEFAULT (preferred)"
+                                   " or SERVER_PORT (deprecated) must be defined.")
+        }
+    }
     try {
         auto envJoinNamespaces = sarus::common::getEnvironmentVariable("JOIN_NAMESPACES");
         joinNamespaces = (boost::algorithm::to_upper_copy(envJoinNamespaces) == std::string("TRUE"));

src/hooks/ssh/SshHook.hpp

@@ -69,7 +69,7 @@ class SshHook {
     pid_t pidOfContainer;
     uid_t uidOfUser;
     gid_t gidOfUser;
-    std::uint16_t serverPort;
+    std::uint16_t serverPort = 0;
     bool overlayMountHostDotSsh = true;
 };
 

src/hooks/ssh/test/test_SSHHook.cpp

@@ -656,6 +656,47 @@ TEST(SSHHookTestGroup, testDefaultServerPort) {
     helper.checkContainerHasSshBinary();
 }
 
+TEST(SSHHookTestGroup, testDefaultServerPortOverridesDeprecatedVar) {
+    std::uint16_t expectedPort = 29476;
+    Helper helper{};
+
+    helper.setRootIds();
+    helper.setupTestEnvironment();
+    sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort));
+
+    // generate + check SSH keys in local repository
+    helper.setUserIds(); // keygen is executed with user privileges
+    SshHook{}.generateSshKeys(true);
+    helper.setRootIds();
+    helper.checkHostHasSshKeys();
+
+    // start sshd
+    helper.writeContainerStateToStdin();
+    SshHook{}.startSshDaemon();
+    helper.checkDefaultSshDaemonPort();
+}
+
+TEST(SSHHookTestGroup, testDeprecatedServerPort) {
+    std::uint16_t expectedPort = 44184;
+    Helper helper{};
+
+    helper.setRootIds();
+    helper.setupTestEnvironment();
+    sarus::common::setEnvironmentVariable("SERVER_PORT", std::to_string(expectedPort));
+    unsetenv("SERVER_PORT_DEFAULT");
+
+    // generate + check SSH keys in local repository
+    helper.setUserIds(); // keygen is executed with user privileges
+    SshHook{}.generateSshKeys(true);
+    helper.setRootIds();
+    helper.checkHostHasSshKeys();
+
+    // start sshd
+    helper.writeContainerStateToStdin();
+    SshHook{}.startSshDaemon();
+    CHECK(helper.getSshDaemonPort() == expectedPort);
+}
+
 TEST(SSHHookTestGroup, testCustomServerPort) {
     std::uint16_t expectedPort = 57864;
     Helper helper{};