mirror of
https://github.com/Sea-Haven-Industries/procurement-ingest.git
synced 2026-09-30 07:13:13 +00:00
Some checks are pending
Deploy / deploy (push) Waiting to run
* feat: deploy-pipeline guards — healthcheck, smoke gate, bundle glob + AST test (refactor phase 0)
Deploys of po-email-processor and workorder-email-processor had no
verification step, so an init-time ImportError in the bundled zip
could ship silently and only surface on the next real S3 event. This
adds a synchronous post-deploy smoke gate wired into the deploy
workflow: both Lambdas are invoked with {"healthcheck": true} and the
FunctionError field is checked, since an Unhandled init error still
returns HTTP 200 on RequestResponse invokes and would false-pass a
plain exit-code check.
The healthcheck branch is the first statement in each handler, before
any boto3/S3 use or ses_auth, and only fires on a top-level direct
invoke ("healthcheck" is not a key AWS ever sets on a real S3
ObjectCreated event, so mail content can't reach this path). It emits
no EMF metrics and no log text that could match the
sender-auth-rejected metric filter, so two deploys in one window
won't trip the alarm.
Separately, the PO stack's asset bundling copied a hand-maintained
four-file allowlist into the zip, so every new sibling module
handler.py imports had to be added by hand or the deploy shipped a
Lambda that ImportErrors at cold start (bit us for template_parser in
PR #105 and nearly for derived_fields in PR #2). Replaced it with a
non-recursive ./*.py glob so top-level source files ship
automatically while tests/ and the stale package/ dir still cannot,
and added an AST-based bundle-consistency test that parses each
handler's first-party imports and fails CI if the bundling command
would omit any of them (a revert to an incomplete allowlist, or code
moved into a subdirectory the glob doesn't cover).
Includes the refactor-evaluation report that scoped this phase.
* fix: review nits — unambiguous bundling-command extraction, smoke payload-parse message, dead asserts
- tests/test_bundle_consistency.py: _extract_bundling_command now collects
all command=[...] matches and demands exactly one per stack file, instead
of silently returning whichever ast.walk visits first if a second bundled
function is ever added.
- scripts/post-deploy-smoke.sh: distinguish an unparseable response payload
from a payload mismatch so the failure message says what actually happened
(the previous "could not parse" branch was unreachable — the inline python
always exited 0).
- test_po_healthcheck.py: drop the substring assertions on stdout that were
dead behind the stricter `captured.out == ""` assertion; keep the stderr
filter-pattern check.
Review follow-up on PR #107; no behavior change to any shipped code path.
250 lines
12 KiB
Python
250 lines
12 KiB
Python
"""AST-based bundle-consistency test for the email-processor Lambdas.
|
|
|
|
Verifies that each pipeline's CDK bundling `command` actually ships every
|
|
first-party sibling module handler.py imports into /asset-output. This
|
|
guards against a regression where the bundling `cp` step (whether an
|
|
explicit filename allowlist or a glob) silently drops a module the handler
|
|
depends on -- history: PR #105 shipped without template_parser.py, and PR #2
|
|
nearly shipped without derived_fields.py, both allowlist-maintenance misses
|
|
that would ImportError at runtime.
|
|
|
|
Pure ast + file reads -- no AWS/boto3/CDK synth, no handler import, no moto.
|
|
Fast and has no dependency on the moto-before-handler import-order invariant
|
|
that the rest of the suite relies on.
|
|
"""
|
|
|
|
import ast
|
|
import re
|
|
from pathlib import Path
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[1]
|
|
|
|
PO_HANDLER = REPO_ROOT / "lambdas" / "po" / "email_processor" / "handler.py"
|
|
WO_HANDLER = REPO_ROOT / "lambdas" / "wo" / "email_processor" / "handler.py"
|
|
PO_STACK = REPO_ROOT / "cdk" / "po_stack.py"
|
|
WO_STACK = REPO_ROOT / "cdk" / "wo_stack.py"
|
|
|
|
# The complete set of top-level modules the PO email_processor ships via the
|
|
# non-recursive `cp ./*.py` glob. Pinned as an upper bound: a new top-level
|
|
# .py in that dir must be added here deliberately, which is the moment to
|
|
# decide whether it SHOULD ship (a real module) or must be excluded (a scratch
|
|
# or secrets file that from_asset would otherwise stage into the bundle on a
|
|
# local deploy). See test_po_bundle_ships_no_unexpected_top_level_modules.
|
|
PO_EXPECTED_TOP_LEVEL_MODULES = frozenset(
|
|
{"handler", "ses_auth", "template_parser", "derived_fields"}
|
|
)
|
|
|
|
|
|
def _first_party_sibling_imports(handler_path: Path) -> set[str]:
|
|
"""Top-level module names handler.py imports that are first-party siblings.
|
|
|
|
Parses only top-level (module-body) `import X` / `from X import ...`
|
|
statements -- not imports nested in functions -- and keeps a name only
|
|
if `<sibling_dir>/X.py` exists, which filters out stdlib/third-party
|
|
imports (json, os, boto3, ...) and keeps exactly the modules the
|
|
bundling step is obligated to ship.
|
|
"""
|
|
tree = ast.parse(handler_path.read_text())
|
|
names: set[str] = set()
|
|
for node in tree.body:
|
|
if isinstance(node, ast.Import):
|
|
for alias in node.names:
|
|
names.add(alias.name.split(".")[0])
|
|
elif isinstance(node, ast.ImportFrom):
|
|
if node.module:
|
|
names.add(node.module.split(".")[0])
|
|
sibling_dir = handler_path.parent
|
|
return {name for name in names if (sibling_dir / f"{name}.py").exists()}
|
|
|
|
|
|
def _extract_bundling_command(stack_path: Path) -> str:
|
|
"""Extract the bash -c bundling command string from a CDK stack file.
|
|
|
|
Locates the `command=[...]` keyword argument to BundlingOptions and
|
|
returns its final list element's string value. Adjacent string-literal
|
|
concatenation (used in both stacks to keep the command readable across
|
|
lines, with `#` comments interspersed) is folded by the parser itself,
|
|
so this returns the exact single command string CDK will hand to bash.
|
|
|
|
Each stack currently has exactly one bundled function; if a second one
|
|
is ever added, "the" bundling command becomes ambiguous and every
|
|
assertion in this file needs to pick its target explicitly — so demand
|
|
exactly one match rather than silently returning whichever ast.walk
|
|
happens to visit first.
|
|
"""
|
|
tree = ast.parse(stack_path.read_text())
|
|
commands: list[str] = []
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.keyword) and node.arg == "command":
|
|
list_node = node.value
|
|
if isinstance(list_node, ast.List) and list_node.elts:
|
|
last = list_node.elts[-1]
|
|
if isinstance(last, ast.Constant) and isinstance(last.value, str):
|
|
commands.append(last.value)
|
|
if len(commands) != 1:
|
|
raise AssertionError(
|
|
f"expected exactly one bundling command=[...] list in {stack_path}, "
|
|
f"found {len(commands)} — update this test to select the intended "
|
|
"bundling command explicitly"
|
|
)
|
|
return commands[0]
|
|
|
|
|
|
def _executed_cp_commands(command: str) -> list[str]:
|
|
"""The `cp ...` invocations bash would actually execute, comment-stripped.
|
|
|
|
Splits the bundling command on shell separators (`&&`, `;`, `|`, newline),
|
|
drops any trailing `#` comment from each segment, and returns the segments
|
|
whose command word is `cp`. This is deliberately stricter than a substring
|
|
match: a glob or `cp -r` string that survives only inside a comment
|
|
(e.g. `cp handler.py /asset-output/ # was: cp ./*.py /asset-output/`) is
|
|
NOT returned, because bash would not execute it -- so a revert that strips
|
|
the real copy but leaves the old text commented cannot false-pass.
|
|
"""
|
|
segments = re.split(r"&&|\|\||;|\||\n", command)
|
|
cp_cmds: list[str] = []
|
|
for seg in segments:
|
|
seg = seg.split("#", 1)[0].strip()
|
|
if re.match(r"cp\b", seg):
|
|
cp_cmds.append(seg)
|
|
return cp_cmds
|
|
|
|
|
|
def _bundling_ships_all(command: str, sibling_names: set[str]) -> bool:
|
|
"""True if `command` is guaranteed to ship every name in `sibling_names`.
|
|
|
|
Inspects only the `cp` commands bash actually executes (comments stripped
|
|
via `_executed_cp_commands`). An executed copy ships every top-level .py
|
|
sibling unconditionally in two shapes:
|
|
- a non-recursive glob copy, e.g. `cp ./*.py /asset-output/` (PO);
|
|
- a recursive copy of the WHOLE source dir, e.g. `cp -r . /asset-output/`
|
|
(WO) -- the source operand must be `.`/`./`, so a narrowed recursive
|
|
copy like `cp -r ./package /asset-output/` does NOT qualify.
|
|
Any other executed `cp` is treated as an explicit filename allowlist (the
|
|
legacy PO form) and is safe only if every required `<name>.py` literally
|
|
appears among the copied filenames -- the branch that must reject a
|
|
reverted allowlist missing a sibling.
|
|
"""
|
|
cp_cmds = _executed_cp_commands(command)
|
|
if not cp_cmds:
|
|
return False
|
|
copied_files: set[str] = set()
|
|
for cp in cp_cmds:
|
|
if re.search(r"(?:^|\s)\.?/?\*\.py(?:\s|$)", cp):
|
|
return True
|
|
if re.search(r"cp\s+-r\s+\.\/?\s+/asset-output", cp):
|
|
return True
|
|
# Explicit-allowlist shape: collect the copied source filenames,
|
|
# ignoring any cp flags and the trailing /asset-output destination.
|
|
match = re.search(
|
|
r"cp\s+(?:-\S+\s+)*(.*?)\s*/asset-output/?\"?\s*$", cp.strip()
|
|
)
|
|
if match:
|
|
copied_files.update(match.group(1).split())
|
|
return all(f"{name}.py" in copied_files for name in sibling_names)
|
|
|
|
|
|
def test_po_bundling_ships_all_first_party_siblings():
|
|
siblings = _first_party_sibling_imports(PO_HANDLER)
|
|
# Sanity: PO handler.py is known to import ses_auth, template_parser,
|
|
# and derived_fields as bare-name siblings. If this ever collapses to
|
|
# an empty set, the test below would vacuously pass -- guard against that.
|
|
assert siblings, "expected first-party sibling imports in PO handler.py"
|
|
|
|
command = _extract_bundling_command(PO_STACK)
|
|
|
|
# Pinned per the Phase 0 healthcheck/bundling contract: PO's bundling
|
|
# command must EXECUTE the non-recursive glob, not a hand-maintained
|
|
# allowlist. Checked against the executed cp (comments stripped) so the
|
|
# old glob text surviving only in a comment cannot satisfy this.
|
|
executed_cps = _executed_cp_commands(command)
|
|
assert any(re.search(r"(?:^|\s)\.?/?\*\.py(?:\s|$)", cp) for cp in executed_cps), (
|
|
"cdk/po_stack.py bundling command must EXECUTE the non-recursive glob "
|
|
"'cp ./*.py /asset-output/' so every first-party sibling handler.py "
|
|
f"imports ships automatically. Executed cp commands: {executed_cps}"
|
|
)
|
|
assert _bundling_ships_all(command, siblings), (
|
|
f"cdk/po_stack.py bundling command does not ship all of {sorted(siblings)}: "
|
|
f"{command!r}"
|
|
)
|
|
|
|
|
|
def test_wo_bundling_ships_all_first_party_siblings():
|
|
siblings = _first_party_sibling_imports(WO_HANDLER)
|
|
assert siblings, "expected first-party sibling imports in WO handler.py"
|
|
|
|
command = _extract_bundling_command(WO_STACK)
|
|
|
|
# WO is out of scope for the Phase 0 glob change -- it ships everything
|
|
# via a recursive `cp -r` of the whole source dir, which is a different
|
|
# (broader, not narrower) mechanism that also guarantees every sibling
|
|
# ships. Assert the executed cp recursively copies the WHOLE dir (source
|
|
# `.`/`./`), so a narrowed `cp -r ./package /asset-output/` does not pass.
|
|
executed_cps = _executed_cp_commands(command)
|
|
assert any(
|
|
re.search(r"cp\s+-r\s+\.\/?\s+/asset-output", cp) for cp in executed_cps
|
|
), (
|
|
"cdk/wo_stack.py bundling command is expected to recursively copy the "
|
|
f"whole source dir so every first-party sibling ships. Executed cp "
|
|
f"commands: {executed_cps}"
|
|
)
|
|
assert _bundling_ships_all(command, siblings), (
|
|
f"cdk/wo_stack.py bundling command does not ship all of {sorted(siblings)}: "
|
|
f"{command!r}"
|
|
)
|
|
|
|
|
|
def test_po_bundle_ships_no_unexpected_top_level_modules():
|
|
"""Upper bound: the PO glob ships EXACTLY the expected top-level modules.
|
|
|
|
The sibling-import tests above prove the glob ships every module the
|
|
handler needs (shipped >= required). This proves the other direction
|
|
(shipped <= expected): because `cp ./*.py` copies every top-level .py in
|
|
the dir -- and `Code.from_asset` stages untracked files too (it does not
|
|
honor .gitignore) -- a stray scratch/secrets .py left in this dir would
|
|
silently ship into the production zip on a local deploy. Pinning the set
|
|
forces any new top-level module to be added to
|
|
PO_EXPECTED_TOP_LEVEL_MODULES deliberately, at which point the author
|
|
decides whether it should ship or be excluded from bundling.
|
|
"""
|
|
top_level = {p.stem for p in PO_HANDLER.parent.glob("*.py")}
|
|
assert top_level == set(PO_EXPECTED_TOP_LEVEL_MODULES), (
|
|
"unexpected top-level .py set in lambdas/po/email_processor -- the "
|
|
"'cp ./*.py' glob would ship exactly these into the Lambda zip. "
|
|
f"Found {sorted(top_level)}, expected "
|
|
f"{sorted(PO_EXPECTED_TOP_LEVEL_MODULES)}. If a new module is "
|
|
"intended, add it to PO_EXPECTED_TOP_LEVEL_MODULES; if it is a scratch "
|
|
"or secrets file, remove it (or exclude it from bundling) before deploy."
|
|
)
|
|
|
|
|
|
def test_detection_logic_catches_allowlist_missing_a_sibling():
|
|
"""Unit-level check on `_bundling_ships_all` itself.
|
|
|
|
Simulates the historical regression shape directly: someone reverts the
|
|
PO glob back to an explicit filename allowlist that omits
|
|
derived_fields.py (the near-miss from PR #2). `_bundling_ships_all` must
|
|
detect the gap so that, combined with the "cp ./*.py" pin above,
|
|
test_po_bundling_ships_all_first_party_siblings fails loudly on any such
|
|
revert rather than silently passing.
|
|
"""
|
|
siblings = _first_party_sibling_imports(PO_HANDLER)
|
|
assert "derived_fields" in siblings # sanity: this is the PR #2 near-miss module
|
|
|
|
reverted_allowlist_command = (
|
|
"pip install --platform manylinux2014_aarch64 --only-binary=:all: "
|
|
"-r requirements.txt -t /asset-output && "
|
|
"cp handler.py ses_auth.py template_parser.py /asset-output/"
|
|
)
|
|
|
|
assert not _bundling_ships_all(reverted_allowlist_command, siblings)
|
|
|
|
# Control: the same allowlist shape WITH derived_fields.py added back in
|
|
# is correctly recognized as complete, proving the failure above is
|
|
# about the missing file and not a false-positive-prone regex.
|
|
complete_allowlist_command = (
|
|
"pip install --platform manylinux2014_aarch64 --only-binary=:all: "
|
|
"-r requirements.txt -t /asset-output && "
|
|
"cp handler.py ses_auth.py template_parser.py derived_fields.py /asset-output/"
|
|
)
|
|
assert _bundling_ships_all(complete_allowlist_command, siblings)
|