From 880bec33bb2b2b951e12fac6cea12e2563b01d33 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sun, 30 Aug 2026 12:56:56 +0200 Subject: [PATCH] fix(roles): name the application in values handed to the stack include `vars:` on an `include_role` is in scope for everything the include pulls in, and the expression is evaluated where it is read rather than where it is written. `sys-stk-full` pulls in the proxy and the database, and each of those roles sets `application_id` to its own id, so a value written with `application_id` answers a different question by the time anyone asks for it. It surfaced as a missing key in a role that never declared the value: a repository url written by one app was looked up on `svc-prx-openresty`, and the git task died on an unresolvable argument rather than on anything to do with the url. The seven definitions here are the ones the included chain actually reads, all of them `compose_repository_url` / `compose_repository_ref` sources. Naming the application literally makes each expression answer the same in every scope it travels through. Values that stay inside their own role keep reading `application_id`; nothing downstream evaluates them. `4ce4236585` fixed the other half of the same mechanism a while back, in `meta/services.yml`, where a role-var was referenced from a value the compose role resolves. Same failure, opposite end of the lookup. The lint fixes the form. It judges only the `compose_*` keys, because matching every handed-over key reported thirteen values, six of which nobody downstream reads. Co-Authored-By: Claude Opus 5 (1M context) --- roles/web-app-akaunting/vars/main.yml | 4 +- roles/web-app-bigbluebutton/vars/main.yml | 4 +- roles/web-app-hugo/vars/main.yml | 4 +- roles/web-app-littlejs/vars/main.yml | 4 +- roles/web-app-openproject/vars/main.yml | 4 +- roles/web-app-roulette-wheel/vars/main.yml | 4 +- roles/web-app-taiga/vars/main.yml | 4 +- ...st_stack_vars_resolve_in_their_own_role.py | 119 ++++++++++++++++++ 8 files changed, 133 insertions(+), 14 deletions(-) create mode 100644 tests/lint/ansible/roles/test_stack_vars_resolve_in_their_own_role.py diff --git a/roles/web-app-akaunting/vars/main.yml b/roles/web-app-akaunting/vars/main.yml index f41b227a13..b08964bc27 100644 --- a/roles/web-app-akaunting/vars/main.yml +++ b/roles/web-app-akaunting/vars/main.yml @@ -6,8 +6,8 @@ database_password: "{{ lookup('config', application_id, 'secrets.credentials.dat AKAUNTING_URL: "{{ lookup('tls', application_id, 'url.base') }}" AKAUNTING_IMAGE: "{{ lookup('config', application_id, 'services.akaunting.image') }}" AKAUNTING_VERSION: "{{ lookup('config', application_id, 'services.akaunting.version') }}" -AKAUNTING_REPOSITORY: "{{ lookup('config', application_id, 'services.akaunting.repository') }}" -AKAUNTING_REF: "{{ lookup('config', application_id, 'services.akaunting.ref') }}" +AKAUNTING_REPOSITORY: "{{ lookup('config', 'web-app-akaunting', 'services.akaunting.repository') }}" +AKAUNTING_REF: "{{ lookup('config', 'web-app-akaunting', 'services.akaunting.ref') }}" AKAUNTING_CONTAINER: "{{ lookup('config', application_id, 'services.akaunting.name') }}" AKAUNTING_VOLUME: "{{ lookup('volume', application_id, 'data').name }}" AKAUNTING_COMPANY_NAME: "{{ lookup('config', application_id, 'services.akaunting.company.name') }}" diff --git a/roles/web-app-bigbluebutton/vars/main.yml b/roles/web-app-bigbluebutton/vars/main.yml index 63b8191091..359128cfe8 100644 --- a/roles/web-app-bigbluebutton/vars/main.yml +++ b/roles/web-app-bigbluebutton/vars/main.yml @@ -7,8 +7,8 @@ compose_mode_force: compose domain: "{{ lookup('domain',application_id) }}" http_port: "{{ lookup('config', application_id, 'services.bigbluebutton.ports.local.http') }}" -BBB_REPOSITORY: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.repository') }}" -BBB_REF: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.ref') }}" +BBB_REPOSITORY: "{{ lookup('config', 'web-app-bigbluebutton', 'services.' ~ ('web-app-bigbluebutton' | get_entity_name) ~ '.repository') }}" +BBB_REF: "{{ lookup('config', 'web-app-bigbluebutton', 'services.' ~ ('web-app-bigbluebutton' | get_entity_name) ~ '.ref') }}" _BBB_COTURN_ROLE: "web-svc-coturn" diff --git a/roles/web-app-hugo/vars/main.yml b/roles/web-app-hugo/vars/main.yml index b6b77a2d53..d568e133d9 100644 --- a/roles/web-app-hugo/vars/main.yml +++ b/roles/web-app-hugo/vars/main.yml @@ -10,8 +10,8 @@ HUGO_NGINX_VERSION: "{{ lookup('config', application_id, 'services.hugo.version' HUGO_BUILDER_IMAGE: "{{ lookup('config', application_id, 'services.hugo.builder_image') }}" HUGO_BUILDER_VERSION: "{{ lookup('config', application_id, 'services.hugo.builder_version') }}" -HUGO_SOURCE_REPO: "{{ lookup('config', application_id, 'services.hugo.source_repository') }}" -HUGO_SOURCE_REF: "{{ lookup('config', application_id, 'services.hugo.source_version') }}" +HUGO_SOURCE_REPO: "{{ lookup('config', 'web-app-hugo', 'services.hugo.source_repository') }}" +HUGO_SOURCE_REF: "{{ lookup('config', 'web-app-hugo', 'services.hugo.source_version') }}" HUGO_BASE_URL: "{{ lookup('tls', application_id, 'url.base') }}" HUGO_ENVIRONMENT: "{{ lookup('config', application_id, 'services.hugo.hugo_environment') }}" diff --git a/roles/web-app-littlejs/vars/main.yml b/roles/web-app-littlejs/vars/main.yml index 65d9981b14..0c423d981c 100644 --- a/roles/web-app-littlejs/vars/main.yml +++ b/roles/web-app-littlejs/vars/main.yml @@ -9,8 +9,8 @@ LITTLEJS_HEADLINE: "LittleJS Playground" LITTLEJS_TITLE: "{{ LITTLEJS_HEADLINE }} – {{ DOMAIN_PRIMARY | upper }}" # Clone coordinates — SPOT in meta/services.yml -LITTLEJS_REPOSITORY: "{{ lookup('config', application_id, 'services.littlejs.repository') }}" -LITTLEJS_REF: "{{ lookup('config', application_id, 'services.littlejs.ref') }}" +LITTLEJS_REPOSITORY: "{{ lookup('config', 'web-app-littlejs', 'services.littlejs.repository') }}" +LITTLEJS_REF: "{{ lookup('config', 'web-app-littlejs', 'services.littlejs.ref') }}" # Human-readable upstream link for nav_bottom.html (strip .git suffix) LITTLEJS_REPOSITORY_BASE: "{{ LITTLEJS_REPOSITORY | regex_replace('\\.git$', '') }}" diff --git a/roles/web-app-openproject/vars/main.yml b/roles/web-app-openproject/vars/main.yml index e2cc94111e..a0ad4b5cf9 100644 --- a/roles/web-app-openproject/vars/main.yml +++ b/roles/web-app-openproject/vars/main.yml @@ -4,8 +4,8 @@ entity_name: "{{ application_id | get_entity_name }}" OPENPROJECT_VERSION: "{{ lookup('config', application_id, 'services.web.version') }}" OPENPROJECT_IMAGE: "{{ lookup('config', application_id, 'services.web.image') }}" -OPENPROJECT_REPOSITORY: "{{ lookup('config', application_id, 'services.openproject.repository') }}" -OPENPROJECT_REF: "{{ lookup('config', application_id, 'services.openproject.ref') }}" +OPENPROJECT_REPOSITORY: "{{ lookup('config', 'web-app-openproject', 'services.openproject.repository') }}" +OPENPROJECT_REF: "{{ lookup('config', 'web-app-openproject', 'services.openproject.ref') }}" OPENPROJECT_WEB_SERVICE: "web" OPENPROJECT_WEB_CONTAINER: "{{ lookup('config', application_id, 'services.' ~ OPENPROJECT_WEB_SERVICE ~ '.name') }}" OPENPROJECT_WEB_CONTAINER_ADDRESS: "{{ lookup('container_address', application_id, OPENPROJECT_WEB_SERVICE) }}" diff --git a/roles/web-app-roulette-wheel/vars/main.yml b/roles/web-app-roulette-wheel/vars/main.yml index 56e828926e..21b8572168 100644 --- a/roles/web-app-roulette-wheel/vars/main.yml +++ b/roles/web-app-roulette-wheel/vars/main.yml @@ -5,5 +5,5 @@ entity_name: "{{ application_id | get_entity_name }}" ROULETTE_WHEEL_APP_REL: "services/repository" ROULETTE_IMAGE: "{{ lookup('config', application_id, 'services.roulette-wheel.image') }}" ROULETTE_VERSION: "{{ lookup('config', application_id, 'services.roulette-wheel.version') }}" -ROULETTE_REPOSITORY: "{{ lookup('config', application_id, 'services.roulette-wheel.repository') }}" -ROULETTE_REF: "{{ lookup('config', application_id, 'services.roulette-wheel.ref') }}" +ROULETTE_REPOSITORY: "{{ lookup('config', 'web-app-roulette-wheel', 'services.roulette-wheel.repository') }}" +ROULETTE_REF: "{{ lookup('config', 'web-app-roulette-wheel', 'services.roulette-wheel.ref') }}" diff --git a/roles/web-app-taiga/vars/main.yml b/roles/web-app-taiga/vars/main.yml index 8dfc4350bb..f62f9f5549 100644 --- a/roles/web-app-taiga/vars/main.yml +++ b/roles/web-app-taiga/vars/main.yml @@ -46,8 +46,8 @@ TAIGA_PLUGIN_PIP_INSTALL_ARGS: >- ) | join(' ') }} -TAIGA_REPOSITORY: "{{ lookup('config', application_id, 'services.taiga.repository') }}" -TAIGA_REF: "{{ lookup('config', application_id, 'services.taiga.ref') }}" +TAIGA_REPOSITORY: "{{ lookup('config', 'web-app-taiga', 'services.taiga.repository') }}" +TAIGA_REF: "{{ lookup('config', 'web-app-taiga', 'services.taiga.ref') }}" TAIGA_VERSION: "{{ lookup('config', application_id, 'services.taiga.version') }}" TAIGA_CONFIG_DIRECTORY: "{{ lookup('container', application_id, 'directories.config') }}" TAIGA_CONFIG_APPEND_PATH: "{{ [TAIGA_CONFIG_DIRECTORY, 'taiga-config.append.py'] | path_join }}" diff --git a/tests/lint/ansible/roles/test_stack_vars_resolve_in_their_own_role.py b/tests/lint/ansible/roles/test_stack_vars_resolve_in_their_own_role.py new file mode 100644 index 0000000000..86112166f4 --- /dev/null +++ b/tests/lint/ansible/roles/test_stack_vars_resolve_in_their_own_role.py @@ -0,0 +1,119 @@ +"""Lint: a value handed to a stack include must not read ``application_id``. + +``vars:`` on an ``include_role`` reaches everything the include pulls in, and +the expression is evaluated where it is used rather than where it is written. +``sys-stk-full`` pulls in the proxy and the database, and those roles set +``application_id`` to their own id, so an expression that reads it resolves +against the wrong application by the time anyone asks for it. + +It fails as a missing key in a role that never declared the value: a repository +url written by one app was looked up on ``svc-prx-openresty``, which has no such +service, and the git task died on an unresolvable argument. + +Naming the application literally makes the expression answer the same in every +scope it travels through. + +Only the ``compose_*`` keys are judged, because only those does the included +chain evaluate. A value nobody downstream reads is merely in scope and harmless; +matching every handed-over key instead reported thirteen values, six of them +nothing. + +Suppression (see ``docs/contributing/actions/testing/suppression.md``): + +* ``# nocheck: stack-var-foreign-scope`` on, or directly above, the definition. +""" + +from __future__ import annotations + +import re +import unittest + +from utils.annotations.suppress import is_suppressed_at +from utils.cache.files import PROJECT_ROOT, read_text +from utils.roles.mapping import ROLE_FILE_VARS_MAIN + +_RULE = "stack-var-foreign-scope" +_STACK_INCLUDE = "sys-stk-full" + +_PASSED_RE = re.compile( + r"^\s+(compose_[a-z_]+):\s*\"\{\{\s*([A-Z][A-Z0-9_]*)\s*\}\}\"" +) +_SCOPED_RE = re.compile(r"(? set[str]: + """Return the variable names a role hands to the stack include. + + Args: + role: the role directory. + """ + passed: set[str] = set() + for path in sorted(role.glob("tasks/**/*.yml")): + lines = read_text(str(path)).splitlines() + for index, line in enumerate(lines): + if _STACK_INCLUDE not in line: + continue + for following in lines[index : index + 12]: + match = _PASSED_RE.match(following) + if match: + passed.add(match.group(2)) + return passed + + +def definitions_reading_scope(role, names: set[str]) -> list[str]: + """Return one finding per handed-over definition that reads the scope. + + Args: + role: the role directory. + names: the variable names the role hands to the stack include. + """ + variables = role / ROLE_FILE_VARS_MAIN + if not names or not variables.is_file(): + return [] + findings = [] + lines = read_text(str(variables)).splitlines() + for index, line in enumerate(lines, start=1): + name = line.split(":", 1)[0].strip() + if name not in names or not _SCOPED_RE.search(line): + continue + if is_suppressed_at(lines, index, _RULE): + continue + findings.append( + f"{variables.relative_to(PROJECT_ROOT)}:{index}: {name} is handed to " + f"{_STACK_INCLUDE} and resolves against whichever role reads it" + ) + return findings + + +def stack_vars_reading_foreign_scope() -> list[str]: + """Return one finding per value that would resolve in a foreign role.""" + findings = [] + for role in sorted((PROJECT_ROOT / "roles").iterdir()): + if not role.is_dir(): + continue + findings.extend(definitions_reading_scope(role, passed_into_stack(role))) + return findings + + +class TestStackVarsResolveInTheirOwnRole(unittest.TestCase): + def test_no_handed_over_value_reads_the_reader_s_scope(self) -> None: + findings = stack_vars_reading_foreign_scope() + self.assertEqual( + [], + findings, + f"value(s) that resolve against the wrong role ({len(findings)}):\n" + + "\n".join(f" - {f}" for f in findings), + ) + + def test_the_shape_it_guards_is_actually_in_use(self) -> None: + """Without one, a renamed include would let the rule pass over everything.""" + handing = [ + role + for role in sorted((PROJECT_ROOT / "roles").iterdir()) + if role.is_dir() and passed_into_stack(role) + ] + self.assertGreater(len(handing), 3, "no role hands a value to the stack") + + +if __name__ == "__main__": + unittest.main()