From 26d345d52cd838efd2d14b7371492565f2bfa221 Mon Sep 17 00:00:00 2001 From: David Boreham Date: Thu, 20 Aug 2026 08:01:58 -0600 Subject: [PATCH] Fix parsing environment definitions per issue #290 --- docs/stack-files.md | 2 ++ src/stack/deploy/k8s/helpers.py | 13 +++++++-- tests/unit/test_deploy_helpers.py | 48 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/docs/stack-files.md b/docs/stack-files.md index f32544b..c9f914e 100644 --- a/docs/stack-files.md +++ b/docs/stack-files.md @@ -363,6 +363,8 @@ A service can pick up the same variable from more than one place. Later sources This is the order Docker Compose uses, and it is applied identically for Kubernetes deployments, so a pod file hands its containers the same values whichever target it is deployed to. A pod file that wants a deployment-time value to reach the container can forward it explicitly, e.g. `environment: {SOME_VAR: "${SOME_VAR}"}`. +In the sequence form, a bare `- SOME_VAR` does the same thing: it names a variable to take from the sources +listed above it, and passes nothing at all when none of them set it. The consequence worth knowing is that an inline default beats a value the deployer supplied with `--config`: the composefile wins, and the deployment is wrong rather than failed. So `stack deploy` reports it, naming the service diff --git a/src/stack/deploy/k8s/helpers.py b/src/stack/deploy/k8s/helpers.py index e1142b3..a721e60 100644 --- a/src/stack/deploy/k8s/helpers.py +++ b/src/stack/deploy/k8s/helpers.py @@ -350,9 +350,18 @@ def _expand_shell_vars(raw_val: str, environ=os.environ) -> str: # TODO: handle the case where the same env var is defined in multiple places def envs_from_compose_file(compose_file_envs: Mapping[str, str], environ=os.environ) -> Mapping[str, str]: result = {} - if isinstance(compose_file_envs, CommentedSeq): + if isinstance(compose_file_envs, list): for item in compose_file_envs: - env_var, env_val = item.split("=", 2) + # Only the first "=" separates the name from the value: a value is free to + # contain more of them. An entry with none at all is compose's pass-through + # form, which names a variable to take from the surrounding environment and + # is omitted entirely when that variable is unset. + env_var, separator, env_val = _env_value_to_str(item).partition("=") + if not separator: + if env_var not in environ: + continue + result.update({env_var: _env_value_to_str(environ[env_var])}) + continue expanded_env_val = _expand_shell_vars(env_val, environ) result.update({env_var: expanded_env_val}) else: diff --git a/tests/unit/test_deploy_helpers.py b/tests/unit/test_deploy_helpers.py index d983c28..dc1e0e4 100644 --- a/tests/unit/test_deploy_helpers.py +++ b/tests/unit/test_deploy_helpers.py @@ -18,9 +18,11 @@ import re import pytest +from ruamel.yaml.comments import CommentedSeq from stack.deploy.deploy_util import convert_to_seconds from stack.deploy.deployment_create import _get_mapped_ports, _parse_config_variables +from stack.deploy.k8s.helpers import envs_from_compose_file from stack.init.init import _parse_http_proxy @@ -180,3 +182,49 @@ def test_parse_http_proxy_rejects_non_numeric_port(value): # The path leads: the reversed 'svc:port:path' form must fail rather than reach the spec. with pytest.raises(SystemExit): _parse_http_proxy(value) + + +# --------------------------------------------------------------------------- +# envs_from_compose_file +# --------------------------------------------------------------------------- + + +def _seq(*items): + # The composefile is parsed by ruamel, so a sequence arrives as a CommentedSeq. + seq = CommentedSeq() + seq.extend(items) + return seq + + +def test_envs_from_compose_file_mapping_form(): + assert envs_from_compose_file({"A": "1", "B": True}, {}) == {"A": "1", "B": "true"} + + +def test_envs_from_compose_file_sequence_form(): + assert envs_from_compose_file(_seq("A=1", "B=2"), {}) == {"A": "1", "B": "2"} + + +def test_envs_from_compose_file_value_containing_equals(): + # Only the first "=" separates name from value; the rest belongs to the value. + assert envs_from_compose_file(_seq("URL=a=b"), {}) == {"URL": "a=b"} + + +def test_envs_from_compose_file_pass_through_takes_surrounding_value(): + # The bare form names a variable to pick up from the environment it is merged into. + assert envs_from_compose_file(_seq("SOME_VAR"), {"SOME_VAR": "from-config"}) == {"SOME_VAR": "from-config"} + + +def test_envs_from_compose_file_pass_through_omitted_when_unset(): + # Compose passes nothing at all rather than an empty string, so neither does this. + assert envs_from_compose_file(_seq("SOME_VAR"), {}) == {} + + +def test_envs_from_compose_file_forwarding_form_expands(): + assert envs_from_compose_file(_seq("SOME_VAR=${SOME_VAR}"), {"SOME_VAR": "from-config"}) == { + "SOME_VAR": "from-config" + } + + +def test_envs_from_compose_file_plain_list_form(): + # A sequence that did not come from ruamel is still a sequence. + assert envs_from_compose_file(["A=1", "SOME_VAR"], {"SOME_VAR": "x"}) == {"A": "1", "SOME_VAR": "x"}