diff --git a/modules/interpreter.py b/modules/interpreter.py index cb034a16..76ba648a 100644 --- a/modules/interpreter.py +++ b/modules/interpreter.py @@ -16,6 +16,16 @@ import re from pathlib import Path +# find_replace_values() memo, keyed (value, module). Reset per +# handle_metadata_vars() pass so nothing leaks across CLI runs. +_FRV_CACHE: dict[tuple[str, str], str] = {} +_FRV_STACK: set[tuple[str, str]] = set() +# Cost cap: bound work per attribute so an unresolvable value bails out fast +# instead of hanging (it would otherwise hit the depth>=50 UNKNOWN path anyway). +_FRV_STATE: dict[str, int] = {"calls": 0} +_FRV_MAX_CALLS = 50000 +_FRV_MAX_LEN = 20000 + def _coerce_output_value(val: Any) -> str: """Coerce a Terraform output value to a string for variable substitution.""" @@ -167,10 +177,15 @@ def handle_metadata_vars(tfdata: Dict[str, Any]) -> Dict[str, Any]: Returns: Updated tfdata with resolved metadata variables """ + # Reset memo caches for this pass (shared across all resources/attrs). + _FRV_CACHE.clear() + _FRV_STACK.clear() # Loop through each resource's metadata attributes for resource, attr_list in tfdata["meta_data"].items(): for key, orig_value in attr_list.items(): value = str(orig_value) + _FRV_STATE["calls"] = 0 # per-attribute budget for the cost cap + _FRV_STATE["warned"] = 0 # warn at most once per attribute # Iteratively resolve all variable references. # Track seen values to detect cycles (e.g. local.A → local.B → local.A) # where find_replace_values keeps producing a different string each @@ -549,25 +564,64 @@ def find_replace_values( ) return "UNKNOWN" - # Regex string matching to create lists of different variable markers found - value = helpers.strip_var_curlies(str(varstring)) - # Find all variable types using regex patterns - var_found_list = re.findall(r"var\.[A-Za-z0-9_\-]+", value) - data_found_list = re.findall(r"data\.[A-Za-z0-9_\-\.\[\]]+", value) - varobject_found_list = re.findall(r"var\.[A-Za-z0-9_\-]+\.[A-Za-z0-9_\-]+", value) - local_found_list = re.findall(r"local\.[A-Za-z0-9_\-\.\[\]]+", value) - modulevar_found_list = [ - m.rstrip("[") for m in re.findall(r"module\.[A-Za-z0-9_\-\.\[\]]+", value) - ] - # Replace found variable strings with actual values in order - value = replace_data_values(data_found_list, value, tfdata) - value = replace_module_vars( - modulevar_found_list, value, module, tfdata, recursion_depth - ) - value = replace_var_values( - var_found_list, varobject_found_list, value, module, tfdata - ) - value = replace_local_values(local_found_list, value, module, tfdata) + # Memoize + guard re-entrant resolution FIRST, so cache hits and re-entrant + # calls are free and do NOT consume the cost budget below. replace_module_vars + # re-runs this on the WHOLE string once per nested ref -> O(refs**depth) and + # hangs on large stacks; memoizing (value, module) and short-circuiting + # re-entrant identical calls makes it ~linear. Resolution is a pure function + # of (value, module) here, so caching is safe. + _cache_key = (str(varstring), str(module)) + if _cache_key in _FRV_CACHE: + return _FRV_CACHE[_cache_key] + if _cache_key in _FRV_STACK: # already resolving this exact string; can't progress + return str(varstring) + + # Cost cap: bound per-attribute work so a value the resolver can never fully + # resolve bails out in bounded time (it would otherwise reach the + # recursion_depth>=50 UNKNOWN path). Warn once per attribute if hit, so real + # stacks tripping the guard are visible. + _FRV_STATE["calls"] += 1 + if _FRV_STATE["calls"] > _FRV_MAX_CALLS or len(str(varstring)) > _FRV_MAX_LEN: + if not _FRV_STATE.get("warned"): + click.echo( + click.style( + f" WARNING: variable resolver hit its cost cap " + f"(calls>{_FRV_MAX_CALLS} or len>{_FRV_MAX_LEN}); leaving " + f"'{str(varstring)[:60]}' partially resolved.", + fg="yellow", + ) + ) + _FRV_STATE["warned"] = 1 + return str(varstring) + + _FRV_STACK.add(_cache_key) + try: + # Regex string matching to create lists of different variable markers + value = helpers.strip_var_curlies(str(varstring)) + var_found_list = re.findall(r"var\.[A-Za-z0-9_\-]+", value) + data_found_list = re.findall(r"data\.[A-Za-z0-9_\-\.\[\]]+", value) + varobject_found_list = re.findall( + r"var\.[A-Za-z0-9_\-]+\.[A-Za-z0-9_\-]+", value + ) + local_found_list = re.findall(r"local\.[A-Za-z0-9_\-\.\[\]]+", value) + modulevar_found_list = [ + m.rstrip("[") for m in re.findall(r"module\.[A-Za-z0-9_\-\.\[\]]+", value) + ] + # Replace found variable strings with actual values in order + value = replace_data_values(data_found_list, value, tfdata) + value = replace_module_vars( + modulevar_found_list, value, module, tfdata, recursion_depth + ) + value = replace_var_values( + var_found_list, varobject_found_list, value, module, tfdata + ) + value = replace_local_values(local_found_list, value, module, tfdata) + finally: + # Always clear the in-progress marker, even on exception, so a raised + # error never leaves a stale entry that poisons later resolutions. + _FRV_STACK.discard(_cache_key) + value = value if isinstance(value, str) else str(value) + _FRV_CACHE[_cache_key] = value return value diff --git a/tests/interpreter_perf_unit_test.py b/tests/interpreter_perf_unit_test.py new file mode 100644 index 00000000..809ff6c1 --- /dev/null +++ b/tests/interpreter_perf_unit_test.py @@ -0,0 +1,100 @@ +"""Tests for the near-linear variable resolver guards in modules.interpreter. + +Covers the memoization, re-entrancy guard, and bounded-cost cap added to +find_replace_values (and its per-pass reset in handle_metadata_vars), plus a +regression guard asserting a pathological, deeply-referential attribute resolves +in bounded time rather than blowing up O(refs**depth). +""" + +import os +import sys +import time +import unittest +from unittest.mock import patch + +parent_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) +sys.path.append(parent_dir) + +import modules.interpreter as interp +from modules.interpreter import find_replace_values, handle_metadata_vars + + +class ResolverGuardTests(unittest.TestCase): + def setUp(self): + # Isolate module-level caches between tests. + interp._FRV_CACHE.clear() + interp._FRV_STACK.clear() + interp._FRV_STATE.clear() + interp._FRV_STATE["calls"] = 0 + + def test_normal_local_resolution_unchanged(self): + """A simple local reference still resolves to its value.""" + tfdata = {"all_locals": {"main": {"foo": "bar"}}} + self.assertEqual(find_replace_values("${local.foo}", "main", tfdata).strip(), "bar") + + def test_cache_hit_does_not_consume_budget(self): + """A cached (value, module) returns immediately without spending budget.""" + interp._FRV_CACHE[("${local.foo}", "main")] = "cached" + interp._FRV_STATE["calls"] = 0 + result = find_replace_values("${local.foo}", "main", {"all_locals": {}}) + self.assertEqual(result, "cached") + self.assertEqual(interp._FRV_STATE["calls"], 0) # budget untouched + + def test_reentrant_call_returns_str_unchanged(self): + """A re-entrant identical (value, module) short-circuits and returns a str.""" + interp._FRV_STACK.add(("${module.m.a}", "main")) + result = find_replace_values("${module.m.a}", "main", {}) + self.assertEqual(result, "${module.m.a}") + self.assertIsInstance(result, str) + + def test_stack_marker_removed_on_exception(self): + """An exception mid-resolution must not leave a stale _FRV_STACK entry.""" + key = ("${local.x}", "main") + with patch.object( + interp.helpers, "strip_var_curlies", side_effect=RuntimeError("boom") + ): + with self.assertRaises(RuntimeError): + find_replace_values("${local.x}", "main", {"all_locals": {}}) + self.assertNotIn(key, interp._FRV_STACK) + + def test_cost_cap_calls_bounds_and_warns(self): + """Exceeding the per-attribute call budget returns the input as a str and warns.""" + with patch.object(interp, "_FRV_MAX_CALLS", 0): + with patch.object(interp.click, "echo") as echo: + result = find_replace_values("${local.foo}", "main", {"all_locals": {}}) + self.assertEqual(result, "${local.foo}") + self.assertIsInstance(result, str) + self.assertTrue(echo.called) # a warning was emitted + + def test_cost_cap_length_bounds(self): + """An over-length value bails out unchanged without processing.""" + big = "${local." + "a" * 50 + "}" + with patch.object(interp, "_FRV_MAX_LEN", 5): + result = find_replace_values(big, "main", {"all_locals": {}}) + self.assertEqual(result, big) + + def test_deeply_referential_attribute_is_bounded(self): + """Regression guard: an attribute referencing many mutually-referencing + module outputs must resolve in bounded time (previously O(refs**depth)).""" + n = 10 + refs = " ".join("${module.m1.o%d}" % i for i in range(n)) + outputs = [] + for i in range(n): + others = " ".join( + "${module.m1.o%d}" % j for j in range(n) if j != i + ) + outputs.append({f"o{i}": {"value": "${var.unset} " + others}}) + tfdata = { + "meta_data": {"aws_thing.x": {"module": "main", "config": refs}}, + "all_output": {";m1;/outputs.tf": outputs}, + "all_locals": {"main": {}}, + "variable_map": {"main": {}}, + "module_source_dict": {}, + } + start = time.time() + handle_metadata_vars(tfdata) + self.assertLess(time.time() - start, 10.0) # seconds; patched runs in <1s + + +if __name__ == "__main__": + unittest.main()