diff --git a/docs/stack-files.md b/docs/stack-files.md index ef096cc..1d82257 100644 --- a/docs/stack-files.md +++ b/docs/stack-files.md @@ -364,6 +364,12 @@ This is the order Docker Compose uses, and it is applied identically for Kuberne 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}"}`. +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 +and the key and both values, whenever a key in the deployment's `config.env` is shadowed by an inline literal that +differs from it. A forwarding entry (above) is the fix and is not reported, nor is an inline literal that agrees +with the config value, nor a sequence entry with no value of its own (`- SOME_VAR`). + ### Service Hostnames Every service in a deployment is reachable from every other service by its service name as it appears in the diff --git a/src/stack/deploy/deployment_create.py b/src/stack/deploy/deployment_create.py index 89b6457..d09e9ca 100644 --- a/src/stack/deploy/deployment_create.py +++ b/src/stack/deploy/deployment_create.py @@ -18,6 +18,7 @@ import json import os import random +import re from importlib import util from pathlib import Path @@ -445,6 +446,40 @@ def _remove_secret_environment_literals(service_info, secret_names): service_info["environment"] = [e for e in env if str(e).split("=", 1)[0] not in secret_names] +def _warn_about_shadowed_config(service_name, service_info, config_vars): + # The deployment's config.env is the lowest-precedence env source on both targets + # (see docs/stack-files.md), so an inline literal for the same key wins and the + # value the deployer supplied at init never reaches the container. That is the + # documented behaviour, and compose's, but it is silent, and a stale inline + # default that outranks `--config PUBLIC_BASE_URL=https://...` produces a wrong + # deployment rather than a failed one. So say so, per service and key. + env = service_info.get("environment") + if not env or not config_vars: + return + if isinstance(env, dict): + inline = {str(name): value for name, value in env.items()} + else: + # A sequence entry with no "=" is a pass-through, not a literal: it carries no + # value of its own and so shadows nothing. + inline = dict(str(e).split("=", 1) for e in env if "=" in str(e)) + for name, value in inline.items(): + if name not in config_vars: + continue + value = "" if value is None else str(value) + # The documented way to let a config value through is to forward it, e.g. + # `SOME_VAR: "${SOME_VAR}"`. That is the fix, not an instance of the problem. + if re.search(r"\$\{?" + re.escape(name) + r"\b", value): + continue + if value == str(config_vars[name]): + continue + log_warn( + f"WARN: {service_name}: the composefile sets {name} inline, which overrides the " + f"{name} in this deployment's config.env; the container will see {value!r}, not " + f"{str(config_vars[name])!r}. To let the deployment's value through, write it as " + f'{name}: "${{{name}}}" in the composefile (see docs/stack-files.md).' + ) + + def _write_config_file(spec: Spec, config_env_file: Path): # Note: we want to write an empty file even if we have no config variables with open(config_env_file, "w") as output_file: @@ -660,6 +695,13 @@ def create_operation(deployment_command_context, parsed_spec: Spec | MergedSpec, for service_info in parsed_pod_file.get(constants.services_key, {}).values(): _remove_secret_environment_literals(service_info, list(parsed_spec.get_secrets())) + # On every target: a config value the deployer supplied is outranked by an + # inline literal for the same key, and silence there is what makes it bite. + config_vars = parsed_spec.get_config() + if config_vars: + for service_name, service_info in parsed_pod_file.get(constants.services_key, {}).items(): + _warn_about_shadowed_config(service_name, service_info, config_vars) + # The backup stack fills a gap that only the Docker target has: on # Kubernetes the backup engine is K8up, configured by the deployer from # the same ambient settings, and a backup container deployed there would diff --git a/tests/unit/test_config_shadowing.py b/tests/unit/test_config_shadowing.py new file mode 100644 index 0000000..7d0616b --- /dev/null +++ b/tests/unit/test_config_shadowing.py @@ -0,0 +1,121 @@ +# Copyright © 2026 Bozeman Pass, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. + +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""A deployment config value an inline `environment:` literal outranks. + +The precedence itself is compose's and is not in question (docs/stack-files.md, and +tests/unit/test_k8s_objects.py for the k8s half of it). What is under test here is +that `stack deploy` says so: a `--config` the deployer supplied that cannot reach the +container is the case where a stack comes up wrong rather than failing (issue #281). +""" + +import textwrap + +from conftest import make_stack_from_compose, run_stack + + +POD = """\ + services: + probe: + image: alpine:local + environment: + PUBLIC_BASE_URL: http://localhost + QUIET: unrelated + ports: + - "8080" + db: + image: postgres:local + """ + + +def deploy(tmp_path, isolated_env, pod=POD, config=("PUBLIC_BASE_URL=https://example.com",)): + stack_dir = make_stack_from_compose(tmp_path, textwrap.dedent(pod)) + spec_file = tmp_path / "spec.yml" + args = ["init", "--stack", str(stack_dir), "--output", str(spec_file)] + for entry in config: + args += ["--config", entry] + result = run_stack(args, isolated_env, cwd=tmp_path) + assert result.returncode == 0, f"init failed:\n{result.stdout}\n{result.stderr}" + result = run_stack( + ["deploy", "--spec-file", str(spec_file), "--deployment-dir", str(tmp_path / "deployment")], + isolated_env, + cwd=tmp_path, + ) + assert result.returncode == 0, f"deploy failed:\n{result.stdout}\n{result.stderr}" + return result + + +def test_a_shadowed_config_value_is_reported(tmp_path, isolated_env): + result = deploy(tmp_path, isolated_env) + # Named service and key, both values, and the way out. + assert "probe" in result.stderr + assert "PUBLIC_BASE_URL" in result.stderr + assert "http://localhost" in result.stderr + assert "https://example.com" in result.stderr + assert 'PUBLIC_BASE_URL: "${PUBLIC_BASE_URL}"' in result.stderr + # The deployment is still created: this is the documented behaviour, not an error. + assert "PUBLIC_BASE_URL=https://example.com" in (tmp_path / "deployment" / "config.env").read_text() + + +def test_keys_that_are_not_shadowed_are_not_reported(tmp_path, isolated_env): + # `db` declares nothing inline, and `QUIET` is not a config value at all. + result = deploy(tmp_path, isolated_env) + assert "db:" not in result.stderr + assert "QUIET" not in result.stderr + + +def test_a_forwarded_value_is_not_reported(tmp_path, isolated_env): + # The documented fix, which must not itself look like the problem. + pod = """\ + services: + probe: + image: alpine:local + environment: + PUBLIC_BASE_URL: ${PUBLIC_BASE_URL} + ports: + - "8080" + """ + result = deploy(tmp_path, isolated_env, pod=pod) + assert "PUBLIC_BASE_URL" not in result.stderr + + +def test_a_sequence_entry_with_no_value_is_not_reported(tmp_path, isolated_env): + # `- PUBLIC_BASE_URL` carries no value of its own, so it shadows nothing. + pod = """\ + services: + probe: + image: alpine:local + environment: + - PUBLIC_BASE_URL + ports: + - "8080" + """ + result = deploy(tmp_path, isolated_env, pod=pod) + assert "PUBLIC_BASE_URL" not in result.stderr + + +def test_an_inline_literal_matching_the_config_is_not_reported(tmp_path, isolated_env): + # Same value from both sources: the container sees what the deployer asked for. + pod = """\ + services: + probe: + image: alpine:local + environment: + - PUBLIC_BASE_URL=https://example.com + ports: + - "8080" + """ + result = deploy(tmp_path, isolated_env, pod=pod) + assert "PUBLIC_BASE_URL" not in result.stderr