From c61c0e24b8445a2d02dfcd098dddb71b60cf7ddb Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 16 Nov 2025 12:32:51 +0100 Subject: [PATCH] Improve error/warning messages w.r.t. YAML quoting (#1205) * Remove superfluous conversions/assignments. * Improve messages. --- plugins/connection/docker.py | 4 +++- plugins/connection/docker_api.py | 8 ++++---- plugins/module_utils/_module_container/base.py | 7 +++++-- plugins/modules/docker_compose_v2_exec.py | 7 ++++--- plugins/modules/docker_compose_v2_run.py | 7 ++++--- plugins/modules/docker_container_exec.py | 9 +++++---- plugins/modules/docker_swarm_service.py | 6 ++++-- .../targets/docker_container/tasks/tests/options.yml | 8 ++++++-- 8 files changed, 35 insertions(+), 21 deletions(-) diff --git a/plugins/connection/docker.py b/plugins/connection/docker.py index 99b8f14f..7d4908bf 100644 --- a/plugins/connection/docker.py +++ b/plugins/connection/docker.py @@ -266,7 +266,9 @@ class Connection(ConnectionBase): if not isinstance(val, str): raise AnsibleConnectionFailure( f"Non-string {what.lower()} found for extra_env option. Ambiguous env options must be " - f"wrapped in quotes to avoid them being interpreted. {what}: {val!r}" + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"{what}: {val!r}" ) local_cmd += [ b"-e", diff --git a/plugins/connection/docker_api.py b/plugins/connection/docker_api.py index 3fd1f3e4..26e6ccf0 100644 --- a/plugins/connection/docker_api.py +++ b/plugins/connection/docker_api.py @@ -282,11 +282,11 @@ class Connection(ConnectionBase): if not isinstance(val, str): raise AnsibleConnectionFailure( f"Non-string {what.lower()} found for extra_env option. Ambiguous env options must be " - f"wrapped in quotes to avoid them being interpreted. {what}: {val!r}" + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"{what}: {val!r}" ) - kk = to_text(k, errors="surrogate_or_strict") - vv = to_text(v, errors="surrogate_or_strict") - data["Env"].append(f"{kk}={vv}") + data["Env"].append(f"{k}={v}") if self.get_option("working_dir") is not None: data["WorkingDir"] = self.get_option("working_dir") diff --git a/plugins/module_utils/_module_container/base.py b/plugins/module_utils/_module_container/base.py index 88a5fb57..58030fc8 100644 --- a/plugins/module_utils/_module_container/base.py +++ b/plugins/module_utils/_module_container/base.py @@ -659,7 +659,9 @@ def _preprocess_env( if not isinstance(value, str): module.fail_json( msg="Non-string value found for env option. Ambiguous env options must be " - f"wrapped in quotes to avoid them being interpreted. Key: {name}" + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"Key: {name}" ) final_env[name] = to_text(value, errors="surrogate_or_strict") formatted_env = [] @@ -947,7 +949,8 @@ def _preprocess_log( value = to_text(v, errors="surrogate_or_strict") module.warn( f"Non-string value found for log_options option '{k}'. The value is automatically converted to {value!r}. " - "If this is not correct, or you want to avoid such warnings, please quote the value." + "If this is not correct, or you want to avoid such warnings, please quote the value," + " or explicitly convert the values to strings when templating them." ) v = value options[k] = v diff --git a/plugins/modules/docker_compose_v2_exec.py b/plugins/modules/docker_compose_v2_exec.py index 1ab738a1..dd302bcb 100644 --- a/plugins/modules/docker_compose_v2_exec.py +++ b/plugins/modules/docker_compose_v2_exec.py @@ -210,13 +210,14 @@ class ExecManager(BaseComposeManager): self.stdin += "\n" if self.env is not None: - for name, value in list(self.env.items()): + for name, value in self.env.items(): if not isinstance(value, str): self.fail( "Non-string value found for env option. Ambiguous env options must be " - f"wrapped in quotes to avoid them being interpreted. Key: {name}" + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"Key: {name}" ) - self.env[name] = to_text(value, errors="surrogate_or_strict") def get_exec_cmd(self, dry_run: bool) -> list[str]: args = self.get_base_args(plain_progress=True) + ["exec"] diff --git a/plugins/modules/docker_compose_v2_run.py b/plugins/modules/docker_compose_v2_run.py index 51da9f55..90a87d2f 100644 --- a/plugins/modules/docker_compose_v2_run.py +++ b/plugins/modules/docker_compose_v2_run.py @@ -296,13 +296,14 @@ class ExecManager(BaseComposeManager): self.stdin += "\n" if self.env is not None: - for name, value in list(self.env.items()): + for name, value in self.env.items(): if not isinstance(value, str): self.fail( "Non-string value found for env option. Ambiguous env options must be " - f"wrapped in quotes to avoid them being interpreted. Key: {name}" + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"Key: {name}" ) - self.env[name] = to_text(value, errors="surrogate_or_strict") def get_run_cmd(self, dry_run: bool) -> list[str]: args = self.get_base_args(plain_progress=True) + ["run"] diff --git a/plugins/modules/docker_container_exec.py b/plugins/modules/docker_container_exec.py index 99651f4e..31d03363 100644 --- a/plugins/modules/docker_container_exec.py +++ b/plugins/modules/docker_container_exec.py @@ -221,16 +221,17 @@ def main() -> None: stdin: str | None = client.module.params["stdin"] strip_empty_ends: bool = client.module.params["strip_empty_ends"] tty: bool = client.module.params["tty"] - env: dict[str, t.Any] = client.module.params["env"] + env: dict[str, t.Any] | None = client.module.params["env"] if env is not None: - for name, value in list(env.items()): + for name, value in env.items(): if not isinstance(value, str): client.module.fail_json( msg="Non-string value found for env option. Ambiguous env options must be " - f"wrapped in quotes to avoid them being interpreted. Key: {name}" + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"Key: {name}" ) - env[name] = to_text(value, errors="surrogate_or_strict") if command is not None: argv = shlex.split(command) diff --git a/plugins/modules/docker_swarm_service.py b/plugins/modules/docker_swarm_service.py index 0c4b77b9..1a4201d7 100644 --- a/plugins/modules/docker_swarm_service.py +++ b/plugins/modules/docker_swarm_service.py @@ -914,8 +914,10 @@ def get_docker_environment( for name, value in env.items(): if not isinstance(value, str): raise ValueError( - "Non-string value found for env option. " - f"Ambiguous env options must be wrapped in quotes to avoid YAML parsing. Key: {name}" + "Non-string value found for env option. Ambiguous env options must be " + "wrapped in quotes to avoid them being interpreted when directly specified " + "in YAML, or explicitly converted to strings when the option is templated. " + f"Key: {name}" ) env_dict[name] = str(value) elif env is not None and isinstance(env, list): diff --git a/tests/integration/targets/docker_container/tasks/tests/options.yml b/tests/integration/targets/docker_container/tasks/tests/options.yml index e8492310..e27a4aac 100644 --- a/tests/integration/targets/docker_container/tasks/tests/options.yml +++ b/tests/integration/targets/docker_container/tasks/tests/options.yml @@ -3077,10 +3077,14 @@ that: - log_options_1 is changed - log_options_2 is not changed - - "'Non-string value found for log_options option \\'max-file\\'. The value is automatically converted to \\'5\\'. If this is not correct, or you want to - avoid such warnings, please quote the value.' in (log_options_2.warnings | default([]))" + - message in (log_options_2.warnings | default([])) - log_options_3 is not changed - log_options_4 is changed + vars: + message: >- + Non-string value found for log_options option 'max-file'. The value is automatically converted to '5'. + If this is not correct, or you want to avoid such warnings, please quote the value, + or explicitly convert the values to strings when templating them. #################################################################### ## mac_address #####################################################