Add typing information, 1/2 (#1176)

* Re-enable typing and improve config.

* Make mypy pass.

* Improve settings.

* First batch of types.

* Add more type hints.

* Fixes.

* Format.

* Fix split_port() without returning to previous type chaos.

* Continue with type hints (and ignores).

plugins/connection/docker.py

@@ -118,6 +118,7 @@ import os.path
 import re
 import selectors
 import subprocess
+import typing as t
 from shlex import quote
 
 from ansible.errors import AnsibleConnectionFailure, AnsibleError, AnsibleFileNotFound
@@ -140,8 +141,8 @@ class Connection(ConnectionBase):
     transport = "community.docker.docker"
     has_pipelining = True
 
-    def __init__(self, play_context, new_stdin, *args, **kwargs):
-        super().__init__(play_context, new_stdin, *args, **kwargs)
+    def __init__(self, *args, **kwargs) -> None:
+        super().__init__(*args, **kwargs)
 
         # Note: docker supports running as non-root in some configurations.
         # (For instance, setting the UNIX socket file to be readable and
@@ -152,11 +153,11 @@ class Connection(ConnectionBase):
         # configured to be connected to by root and they are not running as
         # root.
 
-        self._docker_args = []
-        self._container_user_cache = {}
-        self._version = None
-        self.remote_user = None
-        self.timeout = None
+        self._docker_args: list[bytes | str] = []
+        self._container_user_cache: dict[str, str | None] = {}
+        self._version: str | None = None
+        self.remote_user: str | None = None
+        self.timeout: int | float | None = None
 
         # Windows uses Powershell modules
         if getattr(self._shell, "_IS_WINDOWS", False):
@@ -171,12 +172,12 @@ class Connection(ConnectionBase):
                 raise AnsibleError("docker command not found in PATH") from exc
 
     @staticmethod
-    def _sanitize_version(version):
+    def _sanitize_version(version: str) -> str:
         version = re.sub("[^0-9a-zA-Z.]", "", version)
         version = re.sub("^v", "", version)
         return version
 
-    def _old_docker_version(self):
+    def _old_docker_version(self) -> tuple[list[str], str, bytes, int]:
         cmd_args = self._docker_args
 
         old_version_subcommand = ["version"]
@@ -189,7 +190,7 @@ class Connection(ConnectionBase):
 
         return old_docker_cmd, to_native(cmd_output), err, p.returncode
 
-    def _new_docker_version(self):
+    def _new_docker_version(self) -> tuple[list[str], str, bytes, int]:
         # no result yet, must be newer Docker version
         cmd_args = self._docker_args
 
@@ -202,8 +203,7 @@ class Connection(ConnectionBase):
             cmd_output, err = p.communicate()
             return new_docker_cmd, to_native(cmd_output), err, p.returncode
 
-    def _get_docker_version(self):
-
+    def _get_docker_version(self) -> str:
         cmd, cmd_output, err, returncode = self._old_docker_version()
         if returncode == 0:
             for line in to_text(cmd_output, errors="surrogate_or_strict").split("\n"):
@@ -218,7 +218,7 @@ class Connection(ConnectionBase):
 
         return self._sanitize_version(to_text(cmd_output, errors="surrogate_or_strict"))
 
-    def _get_docker_remote_user(self):
+    def _get_docker_remote_user(self) -> str | None:
         """Get the default user configured in the docker container"""
         container = self.get_option("remote_addr")
         if container in self._container_user_cache:
@@ -243,7 +243,7 @@ class Connection(ConnectionBase):
         self._container_user_cache[container] = user
         return user
 
-    def _build_exec_cmd(self, cmd):
+    def _build_exec_cmd(self, cmd: list[bytes | str]) -> list[bytes | str]:
         """Build the local docker exec command to run cmd on remote_host
 
         If remote_user is available and is supported by the docker
@@ -298,7 +298,7 @@ class Connection(ConnectionBase):
 
         return local_cmd
 
-    def _set_docker_args(self):
+    def _set_docker_args(self) -> None:
         # TODO: this is mostly for backwards compatibility, play_context is used as fallback for older versions
         # docker arguments
         del self._docker_args[:]
@@ -308,7 +308,7 @@ class Connection(ConnectionBase):
         if extra_args:
             self._docker_args += extra_args.split(" ")
 
-    def _set_conn_data(self):
+    def _set_conn_data(self) -> None:
         """initialize for the connection, cannot do only in init since all data is not ready at that point"""
 
         self._set_docker_args()
@@ -323,8 +323,7 @@ class Connection(ConnectionBase):
             self.timeout = self._play_context.timeout
 
     @property
-    def docker_version(self):
-
+    def docker_version(self) -> str:
         if not self._version:
             self._set_docker_args()
 
@@ -341,7 +340,7 @@ class Connection(ConnectionBase):
                 )
         return self._version
 
-    def _get_actual_user(self):
+    def _get_actual_user(self) -> str | None:
         if self.remote_user is not None:
             # An explicit user is provided
             if self.docker_version == "dev" or LooseVersion(
@@ -353,7 +352,7 @@ class Connection(ConnectionBase):
             actual_user = self._get_docker_remote_user()
             if actual_user != self.get_option("remote_user"):
                 display.warning(
-                    f'docker {self.docker_version} does not support remote_user, using container default: {actual_user or "?"}'
+                    f"docker {self.docker_version} does not support remote_user, using container default: {actual_user or '?'}"
                 )
             return actual_user
         if self._display.verbosity > 2:
@@ -363,9 +362,9 @@ class Connection(ConnectionBase):
             return self._get_docker_remote_user()
         return None
 
-    def _connect(self, port=None):
+    def _connect(self) -> t.Self:
         """Connect to the container. Nothing to do"""
-        super()._connect()
+        super()._connect()  # type: ignore[safe-super]
         if not self._connected:
             self._set_conn_data()
             actual_user = self._get_actual_user()
@@ -374,13 +373,16 @@ class Connection(ConnectionBase):
                 host=self.get_option("remote_addr"),
             )
             self._connected = True
+        return self
 
-    def exec_command(self, cmd, in_data=None, sudoable=False):
+    def exec_command(
+        self, cmd: str, in_data: bytes | None = None, sudoable: bool = False
+    ) -> tuple[int, bytes, bytes]:
         """Run a command on the docker host"""
 
         self._set_conn_data()
 
-        super().exec_command(cmd, in_data=in_data, sudoable=sudoable)
+        super().exec_command(cmd, in_data=in_data, sudoable=sudoable)  # type: ignore[safe-super]
 
         local_cmd = self._build_exec_cmd([self._play_context.executable, "-c", cmd])
 
@@ -395,6 +397,9 @@ class Connection(ConnectionBase):
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
         ) as p:
+            assert p.stdin is not None
+            assert p.stdout is not None
+            assert p.stderr is not None
             display.debug("done running command with Popen()")
 
             if self.become and self.become.expect_prompt() and sudoable:
@@ -489,10 +494,10 @@ class Connection(ConnectionBase):
             remote_path = os.path.join(os.path.sep, remote_path)
         return os.path.normpath(remote_path)
 
-    def put_file(self, in_path, out_path):
+    def put_file(self, in_path: str, out_path: str) -> None:
         """Transfer a file from local to docker container"""
         self._set_conn_data()
-        super().put_file(in_path, out_path)
+        super().put_file(in_path, out_path)  # type: ignore[safe-super]
         display.vvv(f"PUT {in_path} TO {out_path}", host=self.get_option("remote_addr"))
 
         out_path = self._prefix_login_path(out_path)
@@ -534,10 +539,10 @@ class Connection(ConnectionBase):
                     f"failed to transfer file {to_native(in_path)} to {to_native(out_path)}:\n{to_native(stdout)}\n{to_native(stderr)}"
                 )
 
-    def fetch_file(self, in_path, out_path):
+    def fetch_file(self, in_path: str, out_path: str) -> None:
         """Fetch a file from container to local."""
         self._set_conn_data()
-        super().fetch_file(in_path, out_path)
+        super().fetch_file(in_path, out_path)  # type: ignore[safe-super]
         display.vvv(
             f"FETCH {in_path} TO {out_path}", host=self.get_option("remote_addr")
         )
@@ -596,7 +601,7 @@ class Connection(ConnectionBase):
 
                     if pp.returncode != 0:
                         raise AnsibleError(
-                            f"failed to fetch file {in_path} to {out_path}:\n{stdout}\n{stderr}"
+                            f"failed to fetch file {in_path} to {out_path}:\n{stdout!r}\n{stderr!r}"
                         )
 
         # Rename if needed
@@ -606,11 +611,11 @@ class Connection(ConnectionBase):
                 to_bytes(out_path, errors="strict"),
             )
 
-    def close(self):
+    def close(self) -> None:
         """Terminate the connection. Nothing to do for Docker"""
-        super().close()
+        super().close()  # type: ignore[safe-super]
         self._connected = False
 
-    def reset(self):
+    def reset(self) -> None:
         # Clear container user cache
         self._container_user_cache = {}

plugins/connection/docker_api.py

@@ -107,6 +107,7 @@ options:
 
 import os
 import os.path
+import typing as t
 
 from ansible.errors import AnsibleConnectionFailure, AnsibleFileNotFound
 from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text
@@ -138,6 +139,12 @@ from ansible_collections.community.docker.plugins.plugin_utils._socket_handler i
 )
 
 
+if t.TYPE_CHECKING:
+    from collections.abc import Callable
+
+    _T = t.TypeVar("_T")
+
+
 MIN_DOCKER_API = None
 
 
@@ -150,10 +157,16 @@ class Connection(ConnectionBase):
     transport = "community.docker.docker_api"
     has_pipelining = True
 
-    def _call_client(self, f, not_found_can_be_resource=False):
+    def _call_client(
+        self,
+        f: Callable[[AnsibleDockerClient], _T],
+        not_found_can_be_resource: bool = False,
+    ) -> _T:
+        if self.client is None:
+            raise AssertionError("Client must be present")
         remote_addr = self.get_option("remote_addr")
         try:
-            return f()
+            return f(self.client)
         except NotFound as e:
             if not_found_can_be_resource:
                 raise AnsibleConnectionFailure(
@@ -179,21 +192,21 @@ class Connection(ConnectionBase):
                 f'An unexpected requests error occurred for container "{remote_addr}" when trying to talk to the Docker daemon: {e}'
             )
 
-    def __init__(self, play_context, new_stdin, *args, **kwargs):
-        super().__init__(play_context, new_stdin, *args, **kwargs)
+    def __init__(self, *args, **kwargs) -> None:
+        super().__init__(*args, **kwargs)
 
-        self.client = None
-        self.ids = {}
+        self.client: AnsibleDockerClient | None = None
+        self.ids: dict[str | None, tuple[int, int]] = {}
 
         # Windows uses Powershell modules
         if getattr(self._shell, "_IS_WINDOWS", False):
             self.module_implementation_preferences = (".ps1", ".exe", "")
 
-        self.actual_user = None
+        self.actual_user: str | None = None
 
-    def _connect(self, port=None):
+    def _connect(self) -> Connection:
         """Connect to the container. Nothing to do"""
-        super()._connect()
+        super()._connect()  # type: ignore[safe-super]
         if not self._connected:
             self.actual_user = self.get_option("remote_user")
             display.vvv(
@@ -212,7 +225,7 @@ class Connection(ConnectionBase):
                 # This saves overhead from calling into docker when we do not need to
                 display.vvv("Trying to determine actual user")
                 result = self._call_client(
-                    lambda: self.client.get_json(
+                    lambda client: client.get_json(
                         "/containers/{0}/json", self.get_option("remote_addr")
                     )
                 )
@@ -221,12 +234,19 @@ class Connection(ConnectionBase):
                     if self.actual_user is not None:
                         display.vvv(f"Actual user is '{self.actual_user}'")
 
-    def exec_command(self, cmd, in_data=None, sudoable=False):
+        return self
+
+    def exec_command(
+        self, cmd: str, in_data: bytes | None = None, sudoable: bool = False
+    ) -> tuple[int, bytes, bytes]:
         """Run a command on the docker host"""
 
-        super().exec_command(cmd, in_data=in_data, sudoable=sudoable)
+        super().exec_command(cmd, in_data=in_data, sudoable=sudoable)  # type: ignore[safe-super]
 
-        command = [self._play_context.executable, "-c", to_text(cmd)]
+        if self.client is None:
+            raise AssertionError("Client must be present")
+
+        command = [self._play_context.executable, "-c", cmd]
 
         do_become = self.become and self.become.expect_prompt() and sudoable
 
@@ -277,7 +297,7 @@ class Connection(ConnectionBase):
                 )
 
         exec_data = self._call_client(
-            lambda: self.client.post_json_to_json(
+            lambda client: client.post_json_to_json(
                 "/containers/{0}/exec", self.get_option("remote_addr"), data=data
             )
         )
@@ -286,7 +306,7 @@ class Connection(ConnectionBase):
         data = {"Tty": False, "Detach": False}
         if need_stdin:
             exec_socket = self._call_client(
-                lambda: self.client.post_json_to_stream_socket(
+                lambda client: client.post_json_to_stream_socket(
                     "/exec/{0}/start", exec_id, data=data
                 )
             )
@@ -295,6 +315,8 @@ class Connection(ConnectionBase):
                     display, exec_socket, container=self.get_option("remote_addr")
                 ) as exec_socket_handler:
                     if do_become:
+                        assert self.become is not None
+
                         become_output = [b""]
 
                         def append_become_output(stream_id, data):
@@ -339,7 +361,7 @@ class Connection(ConnectionBase):
                 exec_socket.close()
         else:
             stdout, stderr = self._call_client(
-                lambda: self.client.post_json_to_stream(
+                lambda client: client.post_json_to_stream(
                     "/exec/{0}/start",
                     exec_id,
                     stream=False,
@@ -350,12 +372,12 @@ class Connection(ConnectionBase):
             )
 
         result = self._call_client(
-            lambda: self.client.get_json("/exec/{0}/json", exec_id)
+            lambda client: client.get_json("/exec/{0}/json", exec_id)
         )
 
         return result.get("ExitCode") or 0, stdout or b"", stderr or b""
 
-    def _prefix_login_path(self, remote_path):
+    def _prefix_login_path(self, remote_path: str) -> str:
         """Make sure that we put files into a standard path
 
         If a path is relative, then we need to choose where to put it.
@@ -373,19 +395,23 @@ class Connection(ConnectionBase):
             remote_path = os.path.join(os.path.sep, remote_path)
         return os.path.normpath(remote_path)
 
-    def put_file(self, in_path, out_path):
+    def put_file(self, in_path: str, out_path: str) -> None:
         """Transfer a file from local to docker container"""
-        super().put_file(in_path, out_path)
+        super().put_file(in_path, out_path)  # type: ignore[safe-super]
         display.vvv(f"PUT {in_path} TO {out_path}", host=self.get_option("remote_addr"))
 
+        if self.client is None:
+            raise AssertionError("Client must be present")
+
         out_path = self._prefix_login_path(out_path)
 
         if self.actual_user not in self.ids:
-            dummy, ids, dummy = self.exec_command(b"id -u && id -g")
+            dummy, ids, dummy2 = self.exec_command("id -u && id -g")
             remote_addr = self.get_option("remote_addr")
             try:
-                user_id, group_id = ids.splitlines()
-                self.ids[self.actual_user] = int(user_id), int(group_id)
+                b_user_id, b_group_id = ids.splitlines()
+                user_id, group_id = int(b_user_id), int(b_group_id)
+                self.ids[self.actual_user] = user_id, group_id
                 display.vvvv(
                     f'PUT: Determined uid={user_id} and gid={group_id} for user "{self.actual_user}"',
                     host=remote_addr,
@@ -398,8 +424,8 @@ class Connection(ConnectionBase):
         user_id, group_id = self.ids[self.actual_user]
         try:
             self._call_client(
-                lambda: put_file(
-                    self.client,
+                lambda client: put_file(
+                    client,
                     container=self.get_option("remote_addr"),
                     in_path=in_path,
                     out_path=out_path,
@@ -415,19 +441,22 @@ class Connection(ConnectionBase):
         except DockerFileCopyError as exc:
             raise AnsibleConnectionFailure(to_native(exc)) from exc
 
-    def fetch_file(self, in_path, out_path):
+    def fetch_file(self, in_path: str, out_path: str) -> None:
         """Fetch a file from container to local."""
-        super().fetch_file(in_path, out_path)
+        super().fetch_file(in_path, out_path)  # type: ignore[safe-super]
         display.vvv(
             f"FETCH {in_path} TO {out_path}", host=self.get_option("remote_addr")
         )
 
+        if self.client is None:
+            raise AssertionError("Client must be present")
+
         in_path = self._prefix_login_path(in_path)
 
         try:
             self._call_client(
-                lambda: fetch_file(
-                    self.client,
+                lambda client: fetch_file(
+                    client,
                     container=self.get_option("remote_addr"),
                     in_path=in_path,
                     out_path=out_path,
@@ -443,10 +472,10 @@ class Connection(ConnectionBase):
         except DockerFileCopyError as exc:
             raise AnsibleConnectionFailure(to_native(exc)) from exc
 
-    def close(self):
+    def close(self) -> None:
         """Terminate the connection. Nothing to do for Docker"""
-        super().close()
+        super().close()  # type: ignore[safe-super]
         self._connected = False
 
-    def reset(self):
+    def reset(self) -> None:
         self.ids.clear()

plugins/connection/nsenter.py

@@ -44,7 +44,9 @@ import fcntl
 import os
 import pty
 import selectors
+import shlex
 import subprocess
+import typing as t
 
 import ansible.constants as C
 from ansible.errors import AnsibleError
@@ -63,12 +65,12 @@ class Connection(ConnectionBase):
     transport = "community.docker.nsenter"
     has_pipelining = False
 
-    def __init__(self, *args, **kwargs):
+    def __init__(self, *args, **kwargs) -> None:
         super().__init__(*args, **kwargs)
         self.cwd = None
         self._nsenter_pid = None
 
-    def _connect(self):
+    def _connect(self) -> t.Self:
         self._nsenter_pid = self.get_option("nsenter_pid")
 
         # Because nsenter requires very high privileges, our remote user
@@ -83,12 +85,15 @@ class Connection(ConnectionBase):
             self._connected = True
         return self
 
-    def exec_command(self, cmd, in_data=None, sudoable=True):
-        super().exec_command(cmd, in_data=in_data, sudoable=sudoable)
+    def exec_command(
+        self, cmd: str, in_data: bytes | None = None, sudoable: bool = True
+    ) -> tuple[int, bytes, bytes]:
+        super().exec_command(cmd, in_data=in_data, sudoable=sudoable)  # type: ignore[safe-super]
 
         display.debug("in nsenter.exec_command()")
 
-        executable = C.DEFAULT_EXECUTABLE.split()[0] if C.DEFAULT_EXECUTABLE else None
+        def_executable: str | None = C.DEFAULT_EXECUTABLE  # type: ignore[attr-defined]
+        executable = def_executable.split()[0] if def_executable else None
 
         if not os.path.exists(to_bytes(executable, errors="surrogate_or_strict")):
             raise AnsibleError(
@@ -109,12 +114,8 @@ class Connection(ConnectionBase):
             "--",
         ]
 
-        if isinstance(cmd, (str, bytes)):
-            cmd_parts = nsenter_cmd_parts + [cmd]
-            cmd = to_bytes(" ".join(cmd_parts))
-        else:
-            cmd_parts = nsenter_cmd_parts + cmd
-            cmd = [to_bytes(arg) for arg in cmd_parts]
+        cmd_parts = nsenter_cmd_parts + [cmd]
+        cmd = to_bytes(" ".join(cmd_parts))
 
         display.vvv(f"EXEC {to_text(cmd)}", host=self._play_context.remote_addr)
         display.debug("opening command with Popen()")
@@ -143,6 +144,9 @@ class Connection(ConnectionBase):
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
         ) as p:
+            assert p.stderr is not None
+            assert p.stdin is not None
+            assert p.stdout is not None
             # if we created a master, we can close the other half of the pty now, otherwise master is stdin
             if master is not None:
                 os.close(stdin)
@@ -234,8 +238,8 @@ class Connection(ConnectionBase):
             display.debug("done with nsenter.exec_command()")
             return (p.returncode, stdout, stderr)
 
-    def put_file(self, in_path, out_path):
-        super().put_file(in_path, out_path)
+    def put_file(self, in_path: str, out_path: str) -> None:
+        super().put_file(in_path, out_path)  # type: ignore[safe-super]
 
         in_path = unfrackpath(in_path, basedir=self.cwd)
         out_path = unfrackpath(out_path, basedir=self.cwd)
@@ -245,26 +249,30 @@ class Connection(ConnectionBase):
             with open(to_bytes(in_path, errors="surrogate_or_strict"), "rb") as in_file:
                 in_data = in_file.read()
             rc, dummy_out, err = self.exec_command(
-                cmd=["tee", out_path], in_data=in_data
+                cmd=f"tee {shlex.quote(out_path)}", in_data=in_data
             )
             if rc != 0:
-                raise AnsibleError(f"failed to transfer file to {out_path}: {err}")
+                raise AnsibleError(
+                    f"failed to transfer file to {out_path}: {to_text(err)}"
+                )
         except IOError as e:
             raise AnsibleError(f"failed to transfer file to {out_path}: {e}") from e
 
-    def fetch_file(self, in_path, out_path):
-        super().fetch_file(in_path, out_path)
+    def fetch_file(self, in_path: str, out_path: str) -> None:
+        super().fetch_file(in_path, out_path)  # type: ignore[safe-super]
 
         in_path = unfrackpath(in_path, basedir=self.cwd)
         out_path = unfrackpath(out_path, basedir=self.cwd)
 
         try:
-            rc, out, err = self.exec_command(cmd=["cat", in_path])
+            rc, out, err = self.exec_command(cmd=f"cat {shlex.quote(in_path)}")
             display.vvv(
                 f"FETCH {in_path} TO {out_path}", host=self._play_context.remote_addr
             )
             if rc != 0:
-                raise AnsibleError(f"failed to transfer file to {in_path}: {err}")
+                raise AnsibleError(
+                    f"failed to transfer file to {in_path}: {to_text(err)}"
+                )
             with open(
                 to_bytes(out_path, errors="surrogate_or_strict"), "wb"
             ) as out_file:
@@ -274,6 +282,6 @@ class Connection(ConnectionBase):
                 f"failed to transfer file to {to_native(out_path)}: {e}"
             ) from e
 
-    def close(self):
+    def close(self) -> None:
         """terminate the connection; nothing to do here"""
         self._connected = False