Merge pull request #1504 from mokibit/resolve-required-variable-syntax

Fix variable interpolation in unquoted values (command and entrypoint)

Author: p12tic

newsfragments/fix_command_entrypoint_interpolation.bugfix

@@ -0,0 +1 @@
+Fix variable interpolation in unquoted ``command`` and ``entrypoint`` values.

podman_compose.py

@@ -2097,10 +2097,6 @@ def normalize_service(service: dict[str, Any], sub_dir: str = "") -> dict[str, A
     if "build" in service and "args" in service["build"]:
         if isinstance(build["args"], dict):
             build["args"] = norm_as_list(build["args"])
-    for key in ("command", "entrypoint"):
-        if key in service:
-            if isinstance(service[key], str):
-                service[key] = shlex.split(service[key])
     for key in ("env_file", "security_opt", "volumes"):
         if key not in service:
             continue

tests/integration/interpolation/docker-compose-command-interpolation.yml

@@ -0,0 +1,5 @@
+version: "3.7"
+services:
+    command_test:
+        image: nopush/podman-compose-test
+        command: echo ${CMD_VAR:?CMD_VAR variable missing}

tests/integration/interpolation/test_podman_compose_interpolation.py

@@ -13,6 +13,12 @@ def compose_yaml_path() -> str:
     return os.path.join(os.path.join(test_path(), "interpolation"), "docker-compose.yml")
 
 
+def compose_command_yaml_path() -> str:
+    return os.path.join(
+        os.path.join(test_path(), "interpolation"), "docker-compose-command-interpolation.yml"
+    )
+
+
 class TestComposeInterpolation(unittest.TestCase, RunSubprocessMixin):
     def test_interpolation(self) -> None:
         try:
@@ -86,3 +92,52 @@ def test_required_set_variable_missing(self) -> None:
             1,
         )
         self.assertIn(b"required variable NOT_A_VARIABLE is missing a value: Missing variable", err)
+
+    def test_command_interpolation_unquoted(self) -> None:
+        try:
+            self.run_subprocess_assert_returncode(
+                [
+                    podman_compose_path(),
+                    "-f",
+                    compose_command_yaml_path(),
+                    "up",
+                ],
+                0,
+                {"CMD_VAR": "hello_world"},
+            )
+            output, _ = self.run_subprocess_assert_returncode(
+                [
+                    podman_compose_path(),
+                    "-f",
+                    compose_command_yaml_path(),
+                    "logs",
+                ],
+                0,
+                {"CMD_VAR": "hello_world"},
+            )
+            self.assertIn(b"hello_world", output)
+        finally:
+            self.run_subprocess_assert_returncode(
+                [
+                    podman_compose_path(),
+                    "-f",
+                    compose_command_yaml_path(),
+                    "down",
+                ],
+                0,
+                {"CMD_VAR": "hello_world"},
+            )
+
+    def test_command_interpolation_unquoted_missing_variable(self) -> None:
+        out, err = self.run_subprocess_assert_returncode(
+            [
+                podman_compose_path(),
+                "-f",
+                compose_command_yaml_path(),
+                "up",
+            ],
+            1,
+        )
+        self.assertIn(
+            b"required variable CMD_VAR is missing a value: CMD_VAR variable missing", err
+        )

tests/unit/test_can_merge_build.py

@@ -122,23 +122,23 @@ def test_parse_with_map_merge_into_none(self):
     @parameterized.expand([
         ({}, {"$$$": []}, {"$$$": []}),
         ({"$$$": []}, {}, {"$$$": []}),
-        ({"$$$": []}, {"$$$": "sh-2"}, {"$$$": ["sh-2"]}),
+        ({"$$$": []}, {"$$$": "sh-2"}, {"$$$": "sh-2"}),
         ({"$$$": "sh-2"}, {"$$$": []}, {"$$$": []}),
-        ({}, {"$$$": "sh"}, {"$$$": ["sh"]}),
-        ({"$$$": "sh"}, {}, {"$$$": ["sh"]}),
-        ({"$$$": "sh-1"}, {"$$$": "sh-2"}, {"$$$": ["sh-2"]}),
-        ({"$$$": ["sh-1"]}, {"$$$": "sh-2"}, {"$$$": ["sh-2"]}),
+        ({}, {"$$$": "sh"}, {"$$$": "sh"}),
+        ({"$$$": "sh"}, {}, {"$$$": "sh"}),
+        ({"$$$": "sh-1"}, {"$$$": "sh-2"}, {"$$$": "sh-2"}),
+        ({"$$$": ["sh-1"]}, {"$$$": "sh-2"}, {"$$$": "sh-2"}),
         ({"$$$": "sh-1"}, {"$$$": ["sh-2"]}, {"$$$": ["sh-2"]}),
         ({"$$$": "sh-1"}, {"$$$": ["sh-2", "sh-3"]}, {"$$$": ["sh-2", "sh-3"]}),
         ({"$$$": ["sh-1"]}, {"$$$": ["sh-2", "sh-3"]}, {"$$$": ["sh-2", "sh-3"]}),
         ({"$$$": ["sh-1", "sh-2"]}, {"$$$": ["sh-3", "sh-4"]}, {"$$$": ["sh-3", "sh-4"]}),
         ({}, {"$$$": ["sh-3", "sh      4"]}, {"$$$": ["sh-3", "sh      4"]}),
-        ({"$$$": "sleep infinity"}, {"$$$": "sh"}, {"$$$": ["sh"]}),
-        ({"$$$": "sh"}, {"$$$": "sleep infinity"}, {"$$$": ["sleep", "infinity"]}),
+        ({"$$$": "sleep infinity"}, {"$$$": "sh"}, {"$$$": "sh"}),
+        ({"$$$": "sh"}, {"$$$": "sleep infinity"}, {"$$$": "sleep infinity"}),
         (
             {},
             {"$$$": "bash -c 'sleep infinity'"},
-            {"$$$": ["bash", "-c", "sleep infinity"]},
+            {"$$$": "bash -c 'sleep infinity'"},
         ),
     ])
     def test_parse_compose_file_when_multiple_composes_keys_command_entrypoint(

tests/unit/test_container_to_args.py

@@ -1317,3 +1317,41 @@ async def test_stop_grace_period_minutes(self) -> None:
                 "busybox",
             ],
         )
+
+    async def test_command_string_is_shlex_split(self) -> None:
+        c = create_compose_mock()
+
+        cnt = get_minimal_container()
+        cnt["command"] = "sleep infinity"
+
+        args = await container_to_args(c, cnt)
+        self.assertEqual(
+            args,
+            [
+                "--name=project_name_service_name1",
+                "-d",
+                "--network=bridge:alias=service_name",
+                "busybox",
+                "sleep",
+                "infinity",
+            ],
+        )
+
+    async def test_entrypoint_string_is_shlex_split(self) -> None:
+        c = create_compose_mock()
+
+        cnt = get_minimal_container()
+        cnt["entrypoint"] = "bash -c 'echo hello'"
+
+        args = await container_to_args(c, cnt)
+        self.assertEqual(
+            args,
+            [
+                "--name=project_name_service_name1",
+                "-d",
+                "--network=bridge:alias=service_name",
+                "--entrypoint",
+                '["bash", "-c", "echo hello"]',
+                "busybox",
+            ],
+        )

tests/unit/test_normalize_service.py

@@ -100,14 +100,16 @@ def test_normalize_service_with_sub_dir(
         ([], []),
         (["sh"], ["sh"]),
         (["sh", "-c", "date"], ["sh", "-c", "date"]),
-        ("sh", ["sh"]),
-        ("sleep infinity", ["sleep", "infinity"]),
+        ("sh", "sh"),
+        ("sleep infinity", "sleep infinity"),
         (
             "bash -c 'sleep infinity'",
-            ["bash", "-c", "sleep infinity"],
+            "bash -c 'sleep infinity'",
         ),
     ])
-    def test_command_like(self, input: Union[list[str], str], expected: list[str]) -> None:
+    def test_command_like(
+        self, input: Union[list[str], str], expected: Union[list[str], str]
+    ) -> None:
         for key in ['command', 'entrypoint']:
             input_service = {}
             input_service[key] = input

tests/unit/test_var_interpolate.py

@@ -177,6 +177,20 @@ class TestVarInterpolate(unittest.TestCase):
         # 45. Dollar sign not followed by valid variable name (should be treated as literal)
         # Disable shell comparison since POSIX shells would treat $5 as fifth argument
         ("Price is $5", {}, "Price is $5", False),
+        # 46. Required variable with spaces in error message (unquoted command syntax)
+        (
+            "cmd ${BAR:?BAR variable missing}",
+            {"BAR": "myvalue"},
+            "cmd myvalue",
+            True,
+        ),
+        # 47. Required variable with spaces in error message fails when missing
+        (
+            "cmd ${BAR:?BAR variable missing}",
+            {},
+            ValueError("required variable BAR is missing a value: BAR variable missing"),
+            True,
+        ),
     ]
 
     @parameterized.expand(test_cases)