refactor: centralize deploy-time substitutions (steps 1-2)
Deploy-time placeholders were spread across four independent
implementations: sed in the Gitea deploy script, ansible.builtin.replace
in two playbooks, a hardcoded suffix in bin/sls_hla_launch_gui.sh, and
another in agebd/pv.py. Each defined its own values, so nothing kept them
in sync and a missed substitution failed silently.
Step 1 -- one source of truth:
* config/environments.yml declares every deploy-time token, its value
per environment, and where tokens are allowed to live. An environment
that is missing a token, or defines an undeclared one, is a load-time
error.
* agebd.subs loads it and renders. Rendering is a strict regex over
declared token names rather than a template engine: the rendered
files are EPICS db/substitution files and Qt XML, and an unknown
token must raise rather than survive into the output.
* scripts/ci/check_subs.py enforces the contract in CI: tokens in a
rendered path are declared, declared tokens only appear where they
are rendered or generated, and no pre-migration delimiters survive.
Step 2 -- separate the two substitution stages by delimiter:
* Creation-time (copier, once per new service) keeps {{ token }}.
Deploy-time (every deploy, per environment) becomes @@token@@.
* This drops the {{ '{{ token }}' }} escaping from templates/, which
existed only because one Jinja pass had to emit syntax for a second.
* Angle brackets were tried first and rejected: << >> is not
well-formed XML, and cli parses qt/*.ui with ElementTree.
* The existing sed and ansible consumers are repointed at the new
delimiters so this stays behaviour-preserving. Routing them through
agebd.subs is step 3.
Also fixes, as a side effect of having one implementation:
* The sed pattern '{{ *agebd_env_suffix_upper* }}' quantified 'r'
rather than the space, so it only matched one exact spacing.
* agebd.subs.read_ioc_port compares service names case-insensitively;
ansible/tasks/utils/get-ioc-port.yml still compares CamelCase
registry names against a lowercase input and never matches.
qt/*.ui is declared as a render path but nothing renders it yet -- the
Qt deploy step is still a plain rsync. Wiring that up is step 4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
720e260b3e
commit
cfd3ecad6c
@@ -0,0 +1,131 @@
|
||||
"""
|
||||
Guard the deploy-time substitution contract.
|
||||
|
||||
Every ``@@token@@`` in this repo must be declared in config/environments.yml,
|
||||
and must live somewhere that actually gets rendered. Three things are checked:
|
||||
|
||||
1. Tokens inside a render path are declared.
|
||||
Catches typos, which would otherwise ship the literal placeholder.
|
||||
|
||||
2. Declared tokens only appear in a render, passthrough or reference path.
|
||||
Catches a placeholder added to a file nobody renders -- exactly the
|
||||
regression that put a suffix token into qt/*.ui while the Qt deploy
|
||||
step was a plain rsync.
|
||||
|
||||
3. No pre-migration ``{{ token }}`` form of a deploy-time token survives,
|
||||
in the paths that carry tokens. Catches files missed by the switch to
|
||||
the ``@@token@@`` delimiters. Reference paths are exempt: Ansible and
|
||||
copier own ``{{ }}`` in their own files, and some of their variables
|
||||
share a name with our tokens.
|
||||
|
||||
Files tracked by git plus new, non-ignored files are scanned, so build and
|
||||
cache directories are skipped for free.
|
||||
"""
|
||||
|
||||
import sys
|
||||
|
||||
import git
|
||||
from agebd.subs import (
|
||||
SubstitutionsConfig,
|
||||
find_legacy_tokens,
|
||||
find_tokens,
|
||||
)
|
||||
from agebd.utils import get_git_root
|
||||
|
||||
REPO_ROOT = get_git_root(__file__)
|
||||
|
||||
|
||||
def iter_tracked_text_files(repo_root=REPO_ROOT):
|
||||
"""
|
||||
Yield (relative_path, contents) for every UTF-8 decodable file that is
|
||||
tracked, or new and not gitignored. New files are included so that a bad
|
||||
token is reported before it is committed rather than after.
|
||||
"""
|
||||
repo = git.Repo(repo_root)
|
||||
listing = repo.git.ls_files("--cached", "--others", "--exclude-standard")
|
||||
for rel_path in sorted(set(listing.splitlines())):
|
||||
path = repo_root / rel_path
|
||||
if not path.is_file():
|
||||
continue
|
||||
try:
|
||||
yield rel_path, path.read_text()
|
||||
except (UnicodeDecodeError, OSError):
|
||||
continue
|
||||
|
||||
|
||||
def check_declared(config, rel_path, contents):
|
||||
"""1. Tokens inside a render path must be declared."""
|
||||
if not config.is_render_path(rel_path):
|
||||
return []
|
||||
|
||||
return [
|
||||
f"{rel_path}: '@@{token}@@' is not declared under 'tokens' in "
|
||||
f"{config.environments_file.name}"
|
||||
for token in sorted(find_tokens(contents))
|
||||
if token not in config.tokens
|
||||
]
|
||||
|
||||
|
||||
def carries_tokens(config, rel_path):
|
||||
"""
|
||||
True for paths whose files carry deploy-time tokens as data: they are
|
||||
either rendered, or copied into something that is.
|
||||
"""
|
||||
return config.is_render_path(rel_path) or config.is_passthrough_path(rel_path)
|
||||
|
||||
|
||||
def check_location(config, rel_path, contents):
|
||||
"""2. Declared tokens must sit in a render, passthrough or reference path."""
|
||||
if carries_tokens(config, rel_path) or config.is_reference_path(rel_path):
|
||||
return []
|
||||
|
||||
return [
|
||||
f"{rel_path}: '@@{token}@@' is in a path that is never rendered. In "
|
||||
f"{config.environments_file.name}, add the path to 'locations.render' if it "
|
||||
f"should be substituted at deploy time, or to 'locations.references' if it is "
|
||||
f"part of the substitution machinery. Otherwise remove the token."
|
||||
for token in sorted(find_tokens(contents) & set(config.tokens))
|
||||
]
|
||||
|
||||
|
||||
def check_no_legacy_delimiters(config, rel_path, contents):
|
||||
"""
|
||||
3. No deploy-time token may still use the creation-time delimiters, in the
|
||||
paths that carry tokens. Elsewhere {{ }} belongs to whichever tool owns
|
||||
the file.
|
||||
"""
|
||||
if not carries_tokens(config, rel_path):
|
||||
return []
|
||||
|
||||
return [
|
||||
f"{rel_path}: '{{{{ {token} }}}}' uses the creation-time delimiters for a "
|
||||
f"deploy-time token. Write it as '@@{token}@@'."
|
||||
for token in sorted(find_legacy_tokens(contents) & set(config.tokens))
|
||||
]
|
||||
|
||||
|
||||
CHECKS = (check_declared, check_location, check_no_legacy_delimiters)
|
||||
|
||||
|
||||
def check_subs(repo_root=REPO_ROOT):
|
||||
"""
|
||||
Raise AssertionError listing every violation, or return silently.
|
||||
"""
|
||||
config = SubstitutionsConfig.load(repo_root / "config")
|
||||
|
||||
errors = []
|
||||
for rel_path, contents in iter_tracked_text_files(repo_root):
|
||||
for check in CHECKS:
|
||||
errors.extend(check(config, rel_path, contents))
|
||||
|
||||
if errors:
|
||||
raise AssertionError("Deploy-time substitution problems found:\n\n " + "\n ".join(errors))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
try:
|
||||
check_subs()
|
||||
except AssertionError as e:
|
||||
print(e, file=sys.stderr)
|
||||
sys.exit(1)
|
||||
print("All deploy-time tokens are declared and live in a rendered path.")
|
||||
@@ -0,0 +1,73 @@
|
||||
import pytest
|
||||
from agebd.subs import SubstitutionsConfig
|
||||
from agebd.utils import get_git_root
|
||||
|
||||
from ci.check_subs import (
|
||||
check_declared,
|
||||
check_location,
|
||||
check_no_legacy_delimiters,
|
||||
check_subs,
|
||||
)
|
||||
|
||||
REPO_ROOT = get_git_root(__file__)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def config():
|
||||
return SubstitutionsConfig.load(REPO_ROOT / "config")
|
||||
|
||||
|
||||
def test_repo_satisfies_the_substitution_contract():
|
||||
check_subs()
|
||||
|
||||
|
||||
def test_undeclared_token_in_a_render_path_is_reported(config):
|
||||
errors = check_declared(config, "qt/A_BD_Tune.ui", "AGEBD-TUNE@@agebd_env_sufix_upper@@:AO")
|
||||
|
||||
assert len(errors) == 1
|
||||
assert "agebd_env_sufix_upper" in errors[0]
|
||||
|
||||
|
||||
def test_declared_token_in_a_render_path_is_accepted(config):
|
||||
assert check_declared(config, "qt/A_BD_Tune.ui", "@@agebd_env_suffix_upper@@") == []
|
||||
|
||||
|
||||
def test_token_outside_a_rendered_path_is_reported(config):
|
||||
errors = check_location(config, "bin/start_service.sh", "SUFFIX=@@agebd_env_suffix_upper@@")
|
||||
|
||||
assert len(errors) == 1
|
||||
assert "never rendered" in errors[0]
|
||||
|
||||
|
||||
def test_token_in_a_render_path_is_accepted(config):
|
||||
assert check_location(config, "qt/A_BD_Tune.ui", "@@agebd_env_suffix_upper@@") == []
|
||||
|
||||
|
||||
def test_token_in_a_passthrough_path_is_accepted(config):
|
||||
assert (
|
||||
check_location(
|
||||
config,
|
||||
"templates/service/{{service_name_lower}}/current/ioc/x.jinja",
|
||||
"@@agebd_env_suffix_upper@@",
|
||||
)
|
||||
== []
|
||||
)
|
||||
|
||||
|
||||
def test_legacy_delimiters_are_reported(config):
|
||||
errors = check_no_legacy_delimiters(
|
||||
config, "qt/A_BD_Tune.ui", "AGEBD-TUNE{{ agebd_env_suffix_upper }}:AO"
|
||||
)
|
||||
|
||||
assert len(errors) == 1
|
||||
assert "@@agebd_env_suffix_upper@@" in errors[0]
|
||||
|
||||
|
||||
def test_creation_time_tokens_are_left_alone(config):
|
||||
"""{{ service_name_upper }} is copier's, not ours."""
|
||||
assert (
|
||||
check_no_legacy_delimiters(
|
||||
config, "templates/service/x/current/ioc/y.jinja", "{{ service_name_upper }}"
|
||||
)
|
||||
== []
|
||||
)
|
||||
Generated
+2
@@ -10,6 +10,7 @@ dependencies = [
|
||||
{ name = "gitpython" },
|
||||
{ name = "h5py" },
|
||||
{ name = "pyepics" },
|
||||
{ name = "pyyaml" },
|
||||
{ name = "typer" },
|
||||
]
|
||||
|
||||
@@ -18,6 +19,7 @@ requires-dist = [
|
||||
{ name = "gitpython", specifier = ">=3.1.50" },
|
||||
{ name = "h5py", specifier = ">=3.16.0" },
|
||||
{ name = "pyepics", specifier = ">=3.5.10" },
|
||||
{ name = "pyyaml", specifier = ">=6.0.3" },
|
||||
{ name = "typer", specifier = ">=0.23.2" },
|
||||
]
|
||||
|
||||
|
||||
Reference in New Issue
Block a user