driver-ssh: pass options dataclass to run

Add SSHCommandRunOptions class and modify the run
method signature to use it. This will make it easier
to change the options before having to change the
signature in the future.

Signed-off-by: Albert Esteve <aesteve@redhat.com>

Author: aesteve-rh

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py

@@ -29,6 +29,14 @@ def from_completed_process(result: subprocess.CompletedProcess) -> "SSHCommandRu
         )
 
 
+@dataclass
+class SSHCommandRunOptions:
+    """Options for running an SSH command"""
+    direct: bool = False
+    capture_output: bool = True
+    capture_as_text: bool = True
+
+
 @dataclass(kw_only=True)
 class SSHWrapperClient(CompositeClient):
     """
@@ -46,7 +54,15 @@ def cli(self):
         @click.option("--direct", is_flag=True, help="Use direct TCP address")
         @click.argument("args", nargs=-1)
         def ssh(direct, args):
-            result = self.run(direct, args)
+            options = SSHCommandRunOptions(
+                direct=direct,
+                # When no args are provided, an interactive shell is implied.
+                # In this case, we must not capture stdout/stderr so the shell
+                # can interact with the terminal.
+                capture_output=bool(args),
+            )
+
+            result = self.run(options, args)
             self.logger.debug("SSH exit code: %s", result.return_code)
 
             if result.stdout:
@@ -69,14 +85,14 @@ def stream(self, method="connect"):
     async def stream_async(self, method):
         return await self.tcp.stream_async(method)
 
-    def run(self, direct, args) -> SSHCommandRunResult:
+    def run(self, options: SSHCommandRunOptions, args) -> SSHCommandRunResult:
         """Run SSH command with the given parameters and arguments"""
         # Get SSH command and default username from driver
         ssh_command = self.call("get_ssh_command")
         default_username = self.call("get_default_username")
         ssh_identity = self.call("get_ssh_identity")
 
-        if direct:
+        if options.direct:
             # Use direct TCP address
             try:
                 address = self.tcp.address()  # (format: "tcp://host:port")
@@ -86,10 +102,14 @@ def run(self, direct, args) -> SSHCommandRunResult:
                 if not host or not port:
                     raise ValueError(f"Invalid address format: {address}")
                 self.logger.debug(f"Using direct TCP connection for SSH - host: {host}, port: {port}")
-                return self._run_ssh_local(host, port, ssh_command, default_username, ssh_identity, args)
+                return self._run_ssh_local(host, port, ssh_command, options, default_username, ssh_identity, args)
             except (DriverMethodNotImplemented, ValueError) as e:
                 self.logger.error(f"Direct address connection failed ({e}), falling back to SSH port forwarding")
-                return self.run(False, args)
+                return self.run(SSHCommandRunOptions(
+                    direct=False,
+                    capture_output=options.capture_output,
+                    capture_as_text=options.capture_as_text,
+                ), args)
         else:
             # Use SSH port forwarding (default behavior)
             self.logger.debug("Using SSH port forwarding for SSH connection")
@@ -98,9 +118,9 @@ def run(self, direct, args) -> SSHCommandRunResult:
             ) as addr:
                 host, port = addr
                 self.logger.debug(f"SSH port forward established - host: {host}, port: {port}")
-                return self._run_ssh_local(host, port, ssh_command, default_username, ssh_identity, args)
+                return self._run_ssh_local(host, port, ssh_command, options, default_username, ssh_identity, args)
 
-    def _run_ssh_local(self, host, port, ssh_command, default_username, ssh_identity, args):
+    def _run_ssh_local(self, host, port, ssh_command, options, default_username, ssh_identity, args):
         """Run SSH command with the given host, port, and arguments"""
         # Create temporary identity file if needed
         identity_file = None
@@ -134,7 +154,7 @@ def _run_ssh_local(self, host, port, ssh_command, default_username, ssh_identity
             ssh_args = self._build_final_ssh_command(ssh_args, ssh_options, host, command_args)
 
             # Execute the command
-            return self._execute_ssh_command(ssh_args)
+            return self._execute_ssh_command(ssh_args, options)
         finally:
             # Clean up temporary identity file
             if identity_file:
@@ -234,10 +254,10 @@ def _build_final_ssh_command(self, ssh_args, ssh_options, host, command_args):
         self.logger.debug(f"Running SSH command: {ssh_args}")
         return ssh_args
 
-    def _execute_ssh_command(self, ssh_args) -> SSHCommandRunResult:
+    def _execute_ssh_command(self, ssh_args, options: SSHCommandRunOptions) -> SSHCommandRunResult:
         """Execute the SSH command and return the result"""
         try:
-            result = subprocess.run(ssh_args, capture_output=True, text=True)
+            result = subprocess.run(ssh_args, capture_output=options.capture_output, text=options.capture_as_text)
             return SSHCommandRunResult.from_completed_process(result)
         except FileNotFoundError:
             self.logger.error(

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/driver_test.py

@@ -5,7 +5,7 @@
 import pytest
 from jumpstarter_driver_network.driver import TcpNetwork
 
-from jumpstarter_driver_ssh.client import SSHCommandRunResult
+from jumpstarter_driver_ssh.client import SSHCommandRunOptions, SSHCommandRunResult
 from jumpstarter_driver_ssh.driver import SSHWrapper
 
 from jumpstarter.common.exceptions import ConfigurationError
@@ -55,7 +55,7 @@ def test_ssh_command_with_default_username():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with default username
-            result = client.run(False, ["hostname"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -87,7 +87,7 @@ def test_ssh_command_without_default_username():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command without default username
-            result = client.run(False, ["hostname"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -117,7 +117,7 @@ def test_ssh_command_with_user_override():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with -l flag overriding default username
-            result = client.run(False, ["-l", "myuser", "hostname"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["-l", "myuser", "hostname"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -155,7 +155,7 @@ def test_ssh_command_with_port():
                 mock_adapter.return_value.__exit__.return_value = None
 
                 # Test SSH command with custom port
-                result = client.run(False, ["hostname"])
+                result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
                 assert isinstance(result, SSHCommandRunResult)
 
                 # Verify subprocess.run was called
@@ -193,7 +193,7 @@ def test_ssh_command_with_direct_flag():
             # Mock the tcp.address() method
             with patch.object(client.tcp, 'address', return_value="tcp://192.168.1.100:22"):
                 # Test SSH command with direct flag
-                result = client.run(True, ["hostname"])
+                result = client.run(SSHCommandRunOptions(direct=True), ["hostname"])
                 assert isinstance(result, SSHCommandRunResult)
 
                 # Verify subprocess.run was called
@@ -224,7 +224,7 @@ def test_ssh_command_error_handling():
             mock_run.side_effect = FileNotFoundError("SSH not found")
 
             # Test SSH command error handling
-            result = client.run(False, ["hostname"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Should return error code 127
@@ -245,7 +245,7 @@ def test_ssh_command_with_multiple_ssh_options():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with multiple SSH options
-            result = client.run(False, [
+            result = client.run(SSHCommandRunOptions(direct=False), [
                 "-o", "StrictHostKeyChecking=no", "-i", "/path/to/key", "command", "arg1", "arg2"
             ])
             assert isinstance(result, SSHCommandRunResult)
@@ -283,7 +283,7 @@ def test_ssh_command_with_unknown_option_treated_as_command():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with unknown option
-            result = client.run(False, ["-l", "user", "-unknown", "command", "arg1"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["-l", "user", "-unknown", "command", "arg1"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -317,7 +317,7 @@ def test_ssh_command_with_no_ssh_options():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with no SSH options
-            result = client.run(False, ["command", "arg1", "arg2"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["command", "arg1", "arg2"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -347,7 +347,7 @@ def test_ssh_command_with_command_l_flag_does_not_interfere_with_username_inject
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with -l flag in the command (like ls -la -l ajo)
-            result = client.run(False, ["ls", "-la", "-l", "ajo"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["ls", "-la", "-l", "ajo"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -469,7 +469,7 @@ def test_ssh_command_with_identity_string():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command with identity string
-            result = client.run(False, ["hostname"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -519,7 +519,7 @@ def test_ssh_command_with_identity_file():
                 mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
                 # Test SSH command with identity file
-                result = client.run(False, ["hostname"])
+                result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
                 assert isinstance(result, SSHCommandRunResult)
 
                 # Verify subprocess.run was called
@@ -563,7 +563,7 @@ def test_ssh_command_without_identity():
             mock_run.return_value = MagicMock(returncode=0, stdout="some stdout", stderr="")
 
             # Test SSH command without identity
-            result = client.run(False, ["hostname"])
+            result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
             assert isinstance(result, SSHCommandRunResult)
 
             # Verify subprocess.run was called
@@ -608,7 +608,7 @@ def test_ssh_identity_temp_file_creation_and_cleanup():
                         mock_temp_file.return_value = mock_temp_file_instance
 
                         # Test SSH command with identity
-                        result = client.run(False, ["hostname"])
+                        result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
                         assert isinstance(result, SSHCommandRunResult)
 
                         # Verify temporary file was created
@@ -644,7 +644,7 @@ def test_ssh_identity_temp_file_creation_error():
                 # Test SSH command with identity should raise an error
                 # The exception will be wrapped in an ExceptionGroup due to the context manager
                 with pytest.raises(ExceptionGroup) as exc_info:
-                    client.run(False, ["hostname"])
+                    client.run(SSHCommandRunOptions(direct=False), ["hostname"])
 
                 # Check that the original OSError is in the exception group
                 assert any(isinstance(e, OSError) and "Permission denied" in str(e) for e in exc_info.value.exceptions)
@@ -677,7 +677,7 @@ def test_ssh_identity_temp_file_cleanup_error():
 
                         # Test SSH command with identity - should still succeed but log warning
                         with patch.object(client, 'logger') as mock_logger:
-                            result = client.run(False, ["hostname"])
+                            result = client.run(SSHCommandRunOptions(direct=False), ["hostname"])
                             assert isinstance(result, SSHCommandRunResult)
 
                             # Verify chmod was called