Skip to content

variable.py: _load_from_runbook shallow-copies VariableEntry objects, letting a runbook default clobber a CLI override #4840

Open

Description

Summary

lisa.variable._load_from_runbook() builds its working copy of the
caller-supplied variables like this:

# make a copy to prevent modifying existing dict
current_variables = higher_level_variables.copy()

This is only a shallow dict.copy(): the new dict has its own keys, but
each value is the same VariableEntry object as in the caller's dict — not
an independent copy. Since merge_variables() / VariableEntry.update()
mutate entries in place, resolving a runbook-declared default for a
variable can silently overwrite the caller's (e.g. command-line-overridden)
VariableEntry object before this function returns, even though the
variable's own final reported value ends up correct later (a later, final
re-merge pass in load_variables() re-applies CLI variables on top, but only
after this function has already finished resolving everything else that
depended on the stale value).

Repro scenario

  1. A runbook declares a variable, e.g. vm, with a default value (vm: azlinux3).
  2. The user overrides it on the CLI: -v vm:ubuntu22.
  3. Another variable in the same runbook's variable: list references the
    first one, e.g. a file: variable entry whose path depends on it:
    variable:
      - name: vm
        value: azlinux3
      - name: address
        file: vms/$(vm).yml
  4. Expected: address is loaded from vms/ubuntu22.yml (honoring the CLI
    override).
  5. Actual (bug): address is loaded from vms/azlinux3.yml — the
    runbook-declared default — because _load_from_runbook()'s internal
    resolution loop mutated the shared VariableEntry object for vm back to
    its default value while resolving it, before using it to resolve
    address's file: path. The final value reported for vm itself
    (e.g. in logs/output) is still correctly ubuntu22, because
    load_variables() re-merges CLI variables one more time at the very end —
    but that happens too late for address, which was already resolved from
    the stale value.

This silently corrupts any variable that:

  • has both a runbook-declared default and a CLI/external override, and
  • is referenced via $(name) by another variable resolved in the same
    variable: list (most commonly a file:-sourced variable whose path
    itself contains a $(...) reference).

Fix

Clone each VariableEntry (not just the containing dict) when building the
working copy:

current_variables = {
    name: entry.copy() for name, entry in higher_level_variables.items()
}

VariableEntry already exposes a .copy() method intended for exactly this
purpose. This was implemented and verified locally:

  • All 15 existing selftests/test_variable.py tests still pass.
  • Manually verified that -v vm:<name> now correctly flows through to a
    dependent file: vms/$(vm).yml-style variable for multiple <name>
    values, where previously it only ever resolved to the first/default name's
    file regardless of the override.

Location

lisa/variable.py, function _load_from_runbook, near the top (current_variables = higher_level_variables.copy()).

Activity

  1. AtharvRG commented on Oct 31, 2026

    @AtharvRG

    I'd like to take this. The fix is to make _load_from_runbook() deep-copy the VariableEntry objects rather than relying on the shallow dict.copy() — e.g. {k: copy(v) for k, v in higher_level_variables.items()} or a dedicated VariableEntry.copy() — so the runbook resolution loop can't mutate the caller's entries while resolving dependent file: paths. My plan: reproduce the CLI-override + dependent-file-variable scenario from the report, apply the deep copy at the top of the function, and add a regression test asserting address resolves against the overridden vm value and that the caller's dict is unmodified after the load.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions