mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-06 02:54:26 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
1e88eb9d1d
commit
880bec33bb
8 files changed
+133
-14
No files matched your search
@@ -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') }}"
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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') }}"
|
||||
|
||||
@@ -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$', '') }}"
|
||||
|
||||
|
||||
@@ -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) }}"
|
||||
|
||||
@@ -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') }}"
|
||||
@@ -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 }}"
|
||||
|
||||
@@ -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"(?<![\w.])(application_id|entity_name)\b")
|
||||
|
||||
|
||||
def passed_into_stack(role) -> 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()
|
||||
Reference in new issue
Block a user