diff --git a/obelisk/python/obelisk_py/core/utils/launch_utils.py b/obelisk/python/obelisk_py/core/utils/launch_utils.py index 99c18d47..13dc75fe 100644 --- a/obelisk/python/obelisk_py/core/utils/launch_utils.py +++ b/obelisk/python/obelisk_py/core/utils/launch_utils.py @@ -59,6 +59,51 @@ def load_config_file(file_path: Union[str, Path], package_name: Optional[str] = return _load_config_recursive(abs_file_path, stack=[]) +# Matches ``${VAR}`` references in config string values. Only the braced form is +# expanded (never bare ``$VAR``) so an incidental ``$`` in a value is left untouched. +_ENV_VAR_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") + + +def _expand_env_vars(node: Any, source: Path) -> Any: + """Recursively expand ``${VAR}`` references in every string value of a loaded config. + + Runs on each YAML file right after it is loaded (before ``include:`` processing and merging), + so env vars work in ``include:`` paths, node ``params``, ``sim`` sensor ``config_path`` entries, + and any other string leaf. Expansion happens here, in the launch process, before values are + serialized into ROS parameters, so downstream nodes (Python *and* C++) receive fully-resolved + absolute paths and need no env-var awareness of their own. + + Only the braced ``${VAR}`` form is expanded; a bare ``$`` is left alone. An **undefined** + variable raises ``KeyError`` (naming the offending file) so a typo fails fast at launch rather + than surfacing later as a confusing file-not-found deep inside a node. Non-string scalars pass + through untouched; dict keys are never expanded, only values. + + Parameters: + node: the loaded config value (dict / list / str / scalar) to expand in place-ish. + source: the file the value came from, used only for error messages. + + Returns: + The value with all ``${VAR}`` references in string leaves expanded. + + Raises: + KeyError: if a referenced environment variable is not set. + """ + if isinstance(node, dict): + return {k: _expand_env_vars(v, source) for k, v in node.items()} + if isinstance(node, list): + return [_expand_env_vars(v, source) for v in node] + if isinstance(node, str): + + def _sub(match: "re.Match") -> str: + name = match.group(1) + if name not in os.environ: + raise KeyError(f"Undefined environment variable ${{{name}}} referenced in obelisk config {source}") + return os.environ[name] + + return _ENV_VAR_RE.sub(_sub, node) + return node + + def _resolve_primary_config_path(file_path: Union[str, Path], package_name: Optional[str]) -> Path: """Resolve the primary (top-level) config path: absolute as-is, else under /share/config/.""" file_path = str(file_path) @@ -95,6 +140,11 @@ def _load_config_recursive(abs_file_path: Path, stack: List[Path]) -> Dict: if raw is None: raw = {} + # Expand ${VAR} references before include-processing and merging, so env vars work in + # include paths and in every string leaf (params, sim config_path, etc.). Done per-file + # here (not once on the merged result) so includes are expanded in their own file's right. + raw = _expand_env_vars(raw, abs_file_path) + includes = raw.pop("include", []) or [] if not isinstance(includes, list): raise ValueError( diff --git a/tests/tests_python/tests_core/test_utils/test_launch_utils.py b/tests/tests_python/tests_core/test_utils/test_launch_utils.py index 8fe2f2cd..dd952458 100644 --- a/tests/tests_python/tests_core/test_utils/test_launch_utils.py +++ b/tests/tests_python/tests_core/test_utils/test_launch_utils.py @@ -218,6 +218,73 @@ def test_load_config_absolute_include_path_works(tmp_path: Path) -> None: assert result["control"][0]["pkg"] == "abs_pkg" +# ---------------------------------------------------------------------- # +# ${VAR} environment-variable expansion # +# ---------------------------------------------------------------------- # + + +def test_load_config_expands_env_vars_in_values(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """``${VAR}`` in string values (node params and sim config_path) is expanded at load time.""" + monkeypatch.setenv("TEST_ROOT", "/opt/deploy") + cfg = tmp_path / "cfg.yaml" + cfg.write_text( + "control:\n" + " - pkg: p\n" + " executable: e\n" + " params:\n" + " seq_path: ${TEST_ROOT}/graph/seqs\n" + " sim:\n" + " - ros_parameter: mujoco_setting\n" + " sensor_settings:\n" + " - config_path: ${TEST_ROOT}/scan/depth.yml\n" + ) + result = load_config_file(cfg) + entry = result["control"][0] + assert entry["params"]["seq_path"] == "/opt/deploy/graph/seqs" + assert entry["sim"][0]["sensor_settings"][0]["config_path"] == "/opt/deploy/scan/depth.yml" + + +def test_load_config_env_var_undefined_raises(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """An undefined ``${VAR}`` raises KeyError naming both the variable and the offending file.""" + monkeypatch.delenv("DEFINITELY_UNSET_VAR", raising=False) + cfg = tmp_path / "cfg.yaml" + cfg.write_text("control:\n - pkg: p\n executable: e\n params:\n x: ${DEFINITELY_UNSET_VAR}/y\n") + with pytest.raises(KeyError, match="DEFINITELY_UNSET_VAR"): + load_config_file(cfg) + + +def test_load_config_env_var_leaves_scalars_and_bare_dollar( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Non-string scalars pass through untouched and a bare ``$`` (no braces) is never expanded.""" + monkeypatch.setenv("TEST_ROOT", "/opt/deploy") + cfg = tmp_path / "cfg.yaml" + cfg.write_text( + "control:\n" + " - pkg: p\n" + " executable: e\n" + " params:\n" + " seq_nodes: [5.0, 0.0, 0.0]\n" + " cyclic: false\n" + " note: 'costs $5'\n" + ) + params = load_config_file(cfg)["control"][0]["params"] + assert params["seq_nodes"] == [5.0, 0.0, 0.0] + assert params["cyclic"] is False + assert params["note"] == "costs $5" + + +def test_load_config_env_var_in_include_path(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """``${VAR}`` inside an ``include:`` path is expanded before the include is resolved.""" + leaf = tmp_path / "leaf.yaml" + leaf.write_text("control:\n - pkg: leaf_pkg\n executable: e\n") + monkeypatch.setenv("TEST_INC_DIR", str(tmp_path)) + root = tmp_path / "root.yaml" + root.write_text("include:\n - ${TEST_INC_DIR}/leaf.yaml\n") + result = load_config_file(root) + assert result["control"][0]["pkg"] == "leaf_pkg" + + def test_load_config_dummy_composed_resolves_to_full_stack() -> None: """End-to-end smoke test: dummy_composed.yaml (which uses `include:`) loads into a dict that looks like a single-file config — control + estimation + robot + joystick all present, no