0eed80f3b9
A fresh install could not build its schema. Core revision 7d51_endpoint_logcollections creates endpointlogcollections with a foreign key onto endpointcommands, but PLUGIN_TABLE_OWNERS lists both tables as owned by the endpoints plugin, and only the plugin's 0002 revision creates endpointcommands. Core migrations run before any plugin chain, and a plugin chain only runs once that plugin is enabled, which cannot happen before the schema exists. So there is no ordering in which the core revision can succeed on an empty database. MySQL rejects it with 1824, 'Failed to open the referenced table endpointcommands'. That is the first command the README gives an adopting site, on the branch that exists for sites to adopt. Nothing caught it because the suite runs on sqlite:///:memory: and SQLite accepts a foreign key onto a table that does not exist. The failure is MySQL-only and therefore invisible to every test. Fix: - endpoints revision 0025 creates the table in the plugin chain, after 0002 has created its foreign key target. Guarded, so it is a no-op on sites that already applied the core revision. - 7d51 skips when endpointcommands is absent, which is exactly the fresh install case 0025 then covers. Its downgrade is guarded to match. - EXPECTED_HEAD_REVISION records the new endpoints head. Verified against real MySQL 8.4, not SQLite: on an empty database the README sequence now completes, 7d51 stamps without creating anything, and enabling the endpoints plugin builds endpointlogcollections with endpointlogcollections_ibfk_2 pointing at endpointcommands.
1122 lines
53 KiB
Python
1122 lines
53 KiB
Python
"""Per-plugin Alembic migration-chain guard tests.
|
|
|
|
Ownership cutover (ADR-008): the core Alembic chain created every table that
|
|
exists through its head (`7d16_directoryemployees`), including the plugin
|
|
tables. From that point on, each bundled plugin that owns tables carries its
|
|
own chain under `plugins/<name>/migrations/`. The `0001` migration in each
|
|
chain is a stamp-only no-op anchor: the core chain already built the tables, so
|
|
there is nothing to create; the anchor just gives the plugin chain a base that
|
|
`flask plugin upgrade-all` stamps into `alembic_version_<plugin>`.
|
|
|
|
These tests pin that contract:
|
|
* PLUGIN_TABLE_OWNERS stays in sync with what the models declare.
|
|
* Every plugin that owns tables has a valid single-head chain.
|
|
* The anchor migrations are genuine no-ops.
|
|
* `flask plugin upgrade-all` runs clean on a fresh DB and is idempotent.
|
|
"""
|
|
import ast
|
|
import re
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from shopdb.plugins.alembic_template import (
|
|
PLUGIN_TABLE_OWNERS,
|
|
_get_plugin_metadata,
|
|
)
|
|
|
|
PLUGINS_DIR = Path(__file__).resolve().parent.parent / 'plugins'
|
|
|
|
# Plugins that own tables carry a migration chain; blueprint-only plugins do
|
|
# not. Today every bundled plugin owns tables, so this is the full set.
|
|
TABLE_OWNING_PLUGINS = tuple(sorted(PLUGIN_TABLE_OWNERS))
|
|
|
|
# The ADR-008 cutover froze this exact set of ten plugins whose tables the core
|
|
# chain had already created. Their 0001 revision is a stamp-only no-op anchor.
|
|
# Plugins built AFTER the cutover (e.g. measuringtools) are NOT in this list:
|
|
# their 0001 is a real baseline that genuinely creates their tables, so the
|
|
# no-op assertion must not apply to them. This is a frozen list on purpose - a
|
|
# newly discovered plugin does not silently get treated as a cutover no-op.
|
|
CUTOVER_PLUGINS = (
|
|
'computers', 'employees', 'knowledgebase', 'machines', 'network',
|
|
'notifications', 'printers', 'slides', 'usb', 'warranty',
|
|
)
|
|
|
|
# Expected head revision id per table-owning plugin, so the upgrade-all test can
|
|
# check both the cutover anchors and post-cutover baselines. Cutover plugins
|
|
# stamp '<plugin>0001anchor'; measuringtools stamps its real baseline id.
|
|
EXPECTED_HEAD_REVISION = {plugin: f'{plugin}0001anchor' for plugin in CUTOVER_PLUGINS}
|
|
EXPECTED_HEAD_REVISION['measuringtools'] = 'measuringtools0001baseline'
|
|
# computers adds the Entra/Azure device identity columns (azuredeviceid, then
|
|
# azureobjectid + azuredevicecategory), then the Configuration/Business policy
|
|
# tables, then the break-glass policyactionholds table, on top of its anchor, so
|
|
# its head is not the f-string default. 0006 widens policyactionholds.expiresat
|
|
# to nullable, where null means an indefinite hold that never self-clears.
|
|
EXPECTED_HEAD_REVISION['computers'] = 'computers0006holdindefinite'
|
|
# backups is also post-cutover: its 0001 really creates backuprevisions, and
|
|
# 0002 adds lastseenat - when a config was last CONFIRMED unchanged, which dedup
|
|
# otherwise throws away.
|
|
EXPECTED_HEAD_REVISION['backups'] = 'backups0003clearlastseen'
|
|
# guides is a NEW plugin, so its 0001 builds tables rather than stamping an
|
|
# anchor over core-owned ones.
|
|
EXPECTED_HEAD_REVISION['guides'] = 'guides0001baseline'
|
|
# geenforce adds the content-addressed blob store (manifestblobs) on top of its
|
|
# baseline.
|
|
EXPECTED_HEAD_REVISION['geenforce'] = 'geenforce0004minlib'
|
|
# printers adds the printersupplyalerts crossing-state table on top of its
|
|
# anchor, then the exact INF driver name Add-PrinterDriver needs, a vendor on
|
|
# the driver, and finally printerobservedqueues - what a bay reported it HAS,
|
|
# kept apart from what it was assigned.
|
|
EXPECTED_HEAD_REVISION['printers'] = 'printers0005observedqueues'
|
|
# machines (renamed from equipment) keeps its original anchor id and adds the
|
|
# rename revision on top, so its head is not the f-string default.
|
|
EXPECTED_HEAD_REVISION['machines'] = 'machines0002rename'
|
|
# employees adds the photofilename column on top of its cutover anchor.
|
|
EXPECTED_HEAD_REVISION['employees'] = 'employees0002photo'
|
|
# usb drops the dead usbcheckouts.machineid column on top of its anchor, then
|
|
# adds scannedviruses so an imported external history keeps the scan result -
|
|
# the field a removable-media audit actually asks about - and finally records
|
|
# who handed a drive back, which is not always who took it out.
|
|
EXPECTED_HEAD_REVISION['usb'] = 'usb0004checkinidentity'
|
|
# network links a device to a catalog model, which is where its photo comes
|
|
# from - machines, PCs and printers already had that link.
|
|
EXPECTED_HEAD_REVISION['network'] = 'network0003prefix'
|
|
# printedparts is post-cutover: its 0001 really creates its tables; 0004 adds
|
|
# the per-transaction revision column.
|
|
EXPECTED_HEAD_REVISION['printedparts'] = 'printedparts0004txnrev'
|
|
# Built after the ADR-008 cutover, so its 0001 genuinely creates the table
|
|
# rather than stamping an anchor. Tech Tools owns a table now because secure
|
|
# send needs one; every other tool in that plugin is browser-side.
|
|
EXPECTED_HEAD_REVISION['tools'] = 'tools0001securesend'
|
|
# notifications indexes businessunitid, then adds the per-type grace window and
|
|
# the shared board category, then the fleet-alert columns plus the per-asset
|
|
# acknowledgement table the endpoint agent writes to.
|
|
EXPECTED_HEAD_REVISION['notifications'] = 'notifications0006fleetalerts'
|
|
# warranty adds the proof-of-cover document columns on top of its anchor.
|
|
EXPECTED_HEAD_REVISION['warranty'] = 'warranty0002proof'
|
|
# endpoints is a NEW post-cutover plugin: its 0001 genuinely creates the
|
|
# endpointstates + endpointevents tables the core chain never owned, 0002
|
|
# adds the endpointcommands queue plus the MTConnect state columns, 0003
|
|
# adds the generic command target/targetkind columns, 0004 adds the
|
|
# endpointapplications SBOM snapshot table, 0005 pins a command sha256, 0006
|
|
# seeds the reusable monitor catalog, 0007 ships the shopfloor monitor policy
|
|
# bundle, 0008 adds the warn-only reconciler drift cache column, and 0009 adds
|
|
# the operatorreports table for tray issue reports.
|
|
# 0010 renames the shipped business policy off the originating site's
|
|
# designator on installs that already stamped 0007 (ADR-015).
|
|
# 0011 adds the trayversion cache column to endpointstates.
|
|
# 0012 adds endpointevents.eventrecordid plus the unique index that dedups a
|
|
# replayed Windows event record.
|
|
# 0013 adds the inventory columns.
|
|
# 0014 adds endpointstates.diskhealthjson, and 0015 adds the system telemetry
|
|
# columns (boot info, reboot reasons, Defender health, NIC link state).
|
|
# 0016 adds the Dell update tables.
|
|
# 0017 adds the health rollup: endpointhealthstates, endpointhealthtransitions
|
|
# and endpointmonitorimpacts.
|
|
# 0018 adds endpointlocks: the admin quarantine refusing actions from non-admins
|
|
# on one endpoint, a caller-authority axis separate from the policy hold.
|
|
# 0019 splits the 0007 bundle into one shipped Configuration Policy per monitor
|
|
# and drops the shipped Business Policy, so sites compose their own.
|
|
# 0020 adds endpointapplications.appfingerprint for delta app updates. 0021 and
|
|
# 0022 are retired no-ops (client certificates, removed). 0023 drops their
|
|
# leftover asset columns, renames app_fingerprint, and adds per-PC credentials:
|
|
# endpointcredentials, endpointenrollrequests and endpointmigrationwaves.
|
|
# 0024 adds endpointbootstraptokens: the short-lived, admin-minted tokens behind
|
|
# the one-line bootstrap installer.
|
|
EXPECTED_HEAD_REVISION['endpoints'] = 'endpoints0025logcollections'
|
|
|
|
# Plugins built after the cutover: their 0001 baseline really creates tables the
|
|
# core chain never owned.
|
|
POST_CUTOVER_PLUGINS = tuple(p for p in PLUGIN_TABLE_OWNERS if p not in CUTOVER_PLUGINS)
|
|
|
|
|
|
def _declared_tablenames(plugin: str) -> set:
|
|
"""Scan a plugin's models package for every __tablename__ literal.
|
|
|
|
Static parse (no import) so the test can compare what the code declares
|
|
against PLUGIN_TABLE_OWNERS without side effects.
|
|
"""
|
|
names = set()
|
|
models_dir = PLUGINS_DIR / plugin / 'models'
|
|
if not models_dir.exists():
|
|
return names
|
|
for source in models_dir.glob('*.py'):
|
|
tree = ast.parse(source.read_text())
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Assign):
|
|
targets = [t.id for t in node.targets if isinstance(t, ast.Name)]
|
|
if '__tablename__' in targets and isinstance(node.value, ast.Constant):
|
|
names.add(node.value.value)
|
|
return names
|
|
|
|
|
|
def test_table_owners_match_declared_models():
|
|
"""Every plugin that declares a __tablename__ is registered in
|
|
PLUGIN_TABLE_OWNERS, and vice versa. Catches a new plugin table that
|
|
forgot to update the ownership map."""
|
|
declared = {p.name for p in PLUGINS_DIR.iterdir()
|
|
if p.is_dir() and _declared_tablenames(p.name)}
|
|
assert declared == set(PLUGIN_TABLE_OWNERS), (
|
|
f"PLUGIN_TABLE_OWNERS keys {set(PLUGIN_TABLE_OWNERS)} do not match "
|
|
f"plugins declaring tables {declared}"
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize('plugin', TABLE_OWNING_PLUGINS)
|
|
def test_bundled_plugin_has_table_owner_entry(plugin):
|
|
"""Every table-owning plugin appears in PLUGIN_TABLE_OWNERS with at least
|
|
one table, documenting which tables it contributes to the schema."""
|
|
assert plugin in PLUGIN_TABLE_OWNERS
|
|
assert len(PLUGIN_TABLE_OWNERS[plugin]) > 0
|
|
|
|
|
|
@pytest.mark.parametrize('plugin', TABLE_OWNING_PLUGINS)
|
|
def test_owned_tables_match_declared_models(plugin):
|
|
"""The tables named in PLUGIN_TABLE_OWNERS are exactly the ones the
|
|
plugin's models declare. Catches drift in either direction."""
|
|
assert set(PLUGIN_TABLE_OWNERS[plugin]) == _declared_tablenames(plugin)
|
|
|
|
|
|
@pytest.mark.parametrize('plugin', TABLE_OWNING_PLUGINS)
|
|
def test_plugin_metadata_has_all_owned_tables(plugin, app):
|
|
"""The MetaData filtered to a plugin's owned tables actually contains
|
|
every table named in PLUGIN_TABLE_OWNERS."""
|
|
with app.app_context():
|
|
md = _get_plugin_metadata(plugin)
|
|
owned = set(PLUGIN_TABLE_OWNERS[plugin])
|
|
missing = owned - set(md.tables.keys())
|
|
assert not missing, f"Plugin {plugin}: owned tables not in metadata: {missing}"
|
|
|
|
|
|
@pytest.mark.parametrize('plugin', TABLE_OWNING_PLUGINS)
|
|
def test_plugin_has_migration_chain(plugin):
|
|
"""Every table-owning plugin has a migrations dir with env.py and exactly
|
|
one anchor revision whose down_revision is None (a valid single-root
|
|
chain)."""
|
|
mig = PLUGINS_DIR / plugin / 'migrations'
|
|
assert (mig / 'env.py').exists(), f"{plugin}: missing migrations/env.py"
|
|
versions = sorted((mig / 'versions').glob('*.py'))
|
|
assert versions, f"{plugin}: no version scripts"
|
|
|
|
roots = []
|
|
heads = set()
|
|
down_revisions = set()
|
|
revisions = set()
|
|
for script in versions:
|
|
tree = ast.parse(script.read_text())
|
|
rev = down = None
|
|
found_down = False
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Assign):
|
|
names = [t.id for t in node.targets if isinstance(t, ast.Name)]
|
|
if 'revision' in names and isinstance(node.value, ast.Constant):
|
|
rev = node.value.value
|
|
if 'down_revision' in names:
|
|
found_down = True
|
|
if isinstance(node.value, ast.Constant):
|
|
down = node.value.value
|
|
assert rev, f"{plugin}: {script.name} has no revision id"
|
|
# alembic_version_<plugin>.version_num is VARCHAR(32). A longer id
|
|
# passes every SQLite test and then fails the real deploy on strict
|
|
# MySQL with "Data too long for column 'version_num'", after the
|
|
# migration body has already run. Caught
|
|
# endpoints0019splitmonitorpolicies (33 chars) during a container roll.
|
|
# The CORE chain's alembic_version is VARCHAR(128), so core ids are not
|
|
# bound by this and are not checked here.
|
|
assert len(rev) <= 32, (
|
|
f"{plugin}: {script.name} revision id {rev!r} is {len(rev)} chars; "
|
|
'alembic_version.version_num holds 32'
|
|
)
|
|
assert found_down, f"{plugin}: {script.name} has no down_revision"
|
|
revisions.add(rev)
|
|
if down is None:
|
|
roots.append(rev)
|
|
else:
|
|
down_revisions.add(down)
|
|
heads = revisions - down_revisions
|
|
assert len(roots) == 1, f"{plugin}: expected 1 root revision, got {roots}"
|
|
assert len(heads) == 1, f"{plugin}: chain must have a single head, got {heads}"
|
|
|
|
|
|
@pytest.mark.parametrize('plugin', CUTOVER_PLUGINS)
|
|
def test_anchor_migration_is_noop(plugin):
|
|
"""The 0001 anchor's upgrade() and downgrade() are pure no-ops: no DDL
|
|
operations, just `pass`. The core chain owns the tables at cutover.
|
|
|
|
Scoped to CUTOVER_PLUGINS only. A plugin built after the cutover ships a
|
|
real baseline (measuringtools), which is deliberately NOT a no-op."""
|
|
anchor = PLUGINS_DIR / plugin / 'migrations' / 'versions' / f'0001_{plugin}_anchor.py'
|
|
assert anchor.exists(), f"{plugin}: missing 0001 anchor migration"
|
|
tree = ast.parse(anchor.read_text())
|
|
funcs = {n.name: n for n in ast.walk(tree)
|
|
if isinstance(n, ast.FunctionDef) and n.name in ('upgrade', 'downgrade')}
|
|
assert set(funcs) == {'upgrade', 'downgrade'}, f"{plugin}: anchor missing up/downgrade"
|
|
for name, fn in funcs.items():
|
|
# Body may only be a docstring/comment plus a bare `pass`. No calls.
|
|
calls = [n for n in ast.walk(fn) if isinstance(n, ast.Call)]
|
|
assert not calls, f"{plugin}: anchor {name}() is not a no-op (found calls)"
|
|
|
|
|
|
def test_upgrade_all_on_fresh_db_is_clean_and_idempotent(tmp_path, monkeypatch):
|
|
"""`flask plugin upgrade-all` on a fresh SQLite DB (after the core schema
|
|
is created) stamps every plugin anchor without error, and a second run is
|
|
a no-op. Mirrors the deploy sequence: `flask db upgrade` then
|
|
`flask plugin upgrade-all`."""
|
|
from sqlalchemy import inspect, text
|
|
from shopdb.config import TestingConfig
|
|
from shopdb import create_app
|
|
from shopdb.extensions import db
|
|
from shopdb.plugins import plugin_manager
|
|
|
|
db_file = tmp_path / 'fresh.db'
|
|
url = f'sqlite:///{db_file}'
|
|
|
|
# Point the whole app (db engine + migration manager) at one file DB so the
|
|
# anchor stamps land where the app can read them back.
|
|
monkeypatch.setattr(TestingConfig, 'SQLALCHEMY_DATABASE_URI', url)
|
|
|
|
# create_app repoints the process-wide plugin_manager singleton; snapshot
|
|
# its wiring and restore it so later tests see the session app unchanged.
|
|
saved = (plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
|
plugin_manager.loader, plugin_manager.migration_manager,
|
|
plugin_manager._registered_prefixes)
|
|
try:
|
|
app = create_app('testing')
|
|
with app.app_context():
|
|
db.create_all() # stand in for the core `flask db upgrade`
|
|
|
|
# db.create_all() over-creates: it builds EVERY table registered on
|
|
# the metadata, including post-cutover plugin tables the core chain
|
|
# would never own. Drop those so each post-cutover baseline creates
|
|
# its own tables exactly as it does after a real core upgrade (where
|
|
# the tables are simply absent). The cutover anchors are no-ops, so
|
|
# their create_all-built tables stay put.
|
|
insp0 = inspect(db.engine)
|
|
for plugin in POST_CUTOVER_PLUGINS:
|
|
# Drop by owned name (SQLite tolerates any order with no rows);
|
|
# avoids resolving cross-metadata FKs via sorted_tables.
|
|
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
|
|
if insp0.has_table(tablename):
|
|
db.session.execute(text(f'DROP TABLE {tablename}'))
|
|
db.session.commit()
|
|
|
|
first = app.extensions['plugin_manager'].upgrade_all_plugins()
|
|
second = app.extensions['plugin_manager'].upgrade_all_plugins()
|
|
|
|
# A plugin that owns no tables (Tech Tools: every tool is
|
|
# client-side) carries no chain, so upgrade-all reports it
|
|
# 'no-migrations'. Assert that explicitly rather than letting it
|
|
# widen the 'ok' check, which would also swallow a table-owning
|
|
# plugin whose chain silently went missing.
|
|
schemaless = set(first) - set(PLUGIN_TABLE_OWNERS)
|
|
for name in schemaless:
|
|
assert first[name] == 'no-migrations', (name, first[name])
|
|
assert second[name] == 'no-migrations', (name, second[name])
|
|
|
|
migrated = {name: status for name, status in first.items()
|
|
if name in PLUGIN_TABLE_OWNERS}
|
|
assert set(migrated) == set(PLUGIN_TABLE_OWNERS)
|
|
assert all(status == 'ok' for status in migrated.values()), migrated
|
|
assert all(second[name] == 'ok' for name in PLUGIN_TABLE_OWNERS), second
|
|
|
|
insp = inspect(db.engine)
|
|
for plugin in PLUGIN_TABLE_OWNERS:
|
|
version_table = f'alembic_version_{plugin}'
|
|
assert insp.has_table(version_table), f"missing {version_table}"
|
|
row = db.session.execute(
|
|
text(f'SELECT version_num FROM {version_table}')
|
|
).fetchone()
|
|
assert row and row[0] == EXPECTED_HEAD_REVISION[plugin]
|
|
finally:
|
|
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
|
plugin_manager.loader, plugin_manager.migration_manager,
|
|
plugin_manager._registered_prefixes) = saved
|
|
|
|
|
|
|
|
# EVERY post-cutover chain, not just the single-baseline ones. This was once
|
|
# restricted to plugins whose whole chain is one baseline, on the reasoning that
|
|
# a longer chain meets a second problem - an ALTER whose column is already
|
|
# present - that guarding a create_table does not fix. True, but the conclusion
|
|
# was wrong: the answer is to guard the ALTERs too, not to stop testing the
|
|
# chains that have them. While this was narrowed, printedparts 0003 sat
|
|
# unguarded for a month and would fail upgrade-all on any create-all-built
|
|
# database, taking 0004 down with it, with no test able to see it.
|
|
#
|
|
# The invariant is now the whole one: a post-cutover chain must run to its head
|
|
# against a database whose tables were built from the models and stamped
|
|
# nowhere, and must do it twice.
|
|
|
|
|
|
@pytest.mark.parametrize('plugin', POST_CUTOVER_PLUGINS)
|
|
def test_baseline_skips_tables_that_already_exist(plugin, tmp_path, monkeypatch):
|
|
"""A baseline must stamp, not fail, when its table is already there.
|
|
|
|
Reported: `flask plugin upgrade-all` failed the tools plugin with "table
|
|
securesendsecrets already exists". Every post-cutover baseline was a plain
|
|
op.create_table, correct only while a table's existence and its plugin's
|
|
Alembic stamp can never disagree. They can - anything that builds tables
|
|
from the models (db.create_all) stamps nothing, and a run that creates a
|
|
table then fails before its stamp commits leaves the same split.
|
|
|
|
The other upgrade-all test DROPS these tables before running, so it only
|
|
ever covered the state where they are absent. This is the other state.
|
|
"""
|
|
from sqlalchemy import inspect, text
|
|
from shopdb.config import TestingConfig
|
|
from shopdb import create_app
|
|
from shopdb.extensions import db
|
|
from shopdb.plugins import plugin_manager
|
|
|
|
monkeypatch.setattr(TestingConfig, 'SQLALCHEMY_DATABASE_URI',
|
|
f'sqlite:///{tmp_path / "preexisting.db"}')
|
|
|
|
saved = (plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
|
plugin_manager.loader, plugin_manager.migration_manager,
|
|
plugin_manager._registered_prefixes)
|
|
try:
|
|
app = create_app('testing')
|
|
with app.app_context():
|
|
db.create_all()
|
|
|
|
# Leave THIS plugin's tables in place - that is the whole point -
|
|
# and drop the other post-cutover ones so their own (longer) chains
|
|
# run the way the sibling test already covers.
|
|
insp0 = inspect(db.engine)
|
|
for other in POST_CUTOVER_PLUGINS:
|
|
if other == plugin:
|
|
continue
|
|
for tablename in PLUGIN_TABLE_OWNERS[other]:
|
|
if insp0.has_table(tablename):
|
|
db.session.execute(text(f'DROP TABLE {tablename}'))
|
|
db.session.commit()
|
|
|
|
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
|
|
assert inspect(db.engine).has_table(tablename), (
|
|
f'{tablename} should have been built by create_all; '
|
|
f'this test is meaningless without it')
|
|
|
|
first = app.extensions['plugin_manager'].upgrade_all_plugins()
|
|
assert first[plugin] == 'ok', first[plugin]
|
|
|
|
second = app.extensions['plugin_manager'].upgrade_all_plugins()
|
|
assert second[plugin] == 'ok', second[plugin]
|
|
|
|
insp = inspect(db.engine)
|
|
row = db.session.execute(
|
|
text(f'SELECT version_num FROM alembic_version_{plugin}')
|
|
).fetchone()
|
|
assert row and row[0] == EXPECTED_HEAD_REVISION[plugin]
|
|
|
|
# No second index over the same columns: create_all names indexes by
|
|
# SQLAlchemy's convention and the baseline by this repo's, so a
|
|
# name-keyed guard would have added a duplicate rather than skipped.
|
|
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
|
|
seen = [tuple(ix.get('column_names') or ())
|
|
for ix in insp.get_indexes(tablename)]
|
|
assert len(seen) == len(set(seen)), (
|
|
f'{tablename} has two indexes over the same columns: {seen}')
|
|
finally:
|
|
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
|
plugin_manager.loader, plugin_manager.migration_manager,
|
|
plugin_manager._registered_prefixes) = saved
|
|
|
|
|
|
def test_upgrade_all_seeds_permissions_and_links_them_to_scopes(
|
|
tmp_path, monkeypatch):
|
|
"""`flask plugin upgrade-all` seeds plugin permissions AND relinks scopes.
|
|
|
|
on_install/on_enable fire only on a state transition, so a permission
|
|
declared by a LATER plugin version never reaches an already-installed site
|
|
through them. Upgrade is the only remaining moment. Before this was wired,
|
|
the name existed only after a manual `flask seed permissions`.
|
|
|
|
The scope relink is the half that actually bites. Creating the Permission
|
|
row only makes the name exist - the prebuilt scopes derive their grants from
|
|
the catalog, so a permission seeded without a following seedscopes() belongs
|
|
to nobody but admin. The plugin installs clean and its UI is invisible to
|
|
every non-admin, which reads as a broken plugin rather than a seeding gap.
|
|
"""
|
|
from shopdb.config import TestingConfig
|
|
from shopdb import create_app
|
|
from shopdb.extensions import db
|
|
from shopdb.core.models import Permission, Role
|
|
from shopdb.plugins import plugin_manager
|
|
|
|
db_file = tmp_path / 'perms.db'
|
|
monkeypatch.setattr(TestingConfig, 'SQLALCHEMY_DATABASE_URI',
|
|
f'sqlite:///{db_file}')
|
|
|
|
saved = (plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
|
plugin_manager.loader, plugin_manager.migration_manager,
|
|
plugin_manager._registered_prefixes)
|
|
try:
|
|
app = create_app('testing')
|
|
with app.app_context():
|
|
db.create_all()
|
|
Permission.seed()
|
|
Role.seedscopes()
|
|
db.session.commit()
|
|
|
|
pm = app.extensions['plugin_manager']
|
|
|
|
# Stand in for a permission that a later plugin version declares:
|
|
# delete a plugin-owned view permission so upgrade-all has to put it
|
|
# back the way it would for a genuinely new one.
|
|
target = Permission.query.filter_by(name='endpoints.view').first()
|
|
assert target is not None, 'fixture expects endpoints.view to exist'
|
|
for role in list(target.roles):
|
|
role.permissions.remove(target)
|
|
db.session.delete(target)
|
|
db.session.commit()
|
|
assert Permission.query.filter_by(name='endpoints.view').first() is None
|
|
|
|
pm.upgrade_all_plugins()
|
|
|
|
# The row is back...
|
|
restored = Permission.query.filter_by(name='endpoints.view').first()
|
|
assert restored is not None, (
|
|
'upgrade-all did not re-seed a missing plugin permission')
|
|
|
|
# ...and it is actually granted to the read-capable scopes, not
|
|
# merely present. This is the assertion that fails if seedscopes()
|
|
# is dropped from the upgrade path.
|
|
holders = {role.rolename for role in restored.roles}
|
|
assert 'technician' in holders, (
|
|
f'endpoints.view reseeded but not linked to any scope: {holders}')
|
|
finally:
|
|
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
|
|
plugin_manager.loader, plugin_manager.migration_manager,
|
|
plugin_manager._registered_prefixes) = saved
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# ADR-008 lane guard: a plugin migration may only do DDL on its OWN tables.
|
|
#
|
|
# The endpoints chain broke this and nothing noticed. 0021 (shipped, since
|
|
# rewritten to a no-op stub) added five client-certificate columns plus
|
|
# idx_assets_cert_fingerprint to the CORE assets table from inside a plugin
|
|
# chain, and 0023 drops them again. Both directions are guarded and idempotent
|
|
# now, so the breach self-heals on every database - but self-healing is not
|
|
# prevention, and the only thing that stopped the next one was that somebody
|
|
# happened to read the file. That is what this guard replaces.
|
|
#
|
|
# The rule, stated as ADR-008 states it: the core Alembic chain owns core
|
|
# tables, each plugin chain owns the tables listed for it in
|
|
# PLUGIN_TABLE_OWNERS, and no chain reaches across. `assets` is not in
|
|
# PLUGIN_TABLE_OWNERS['endpoints'] and cannot be, so the per-plugin MetaData
|
|
# filter never saw the columns either: the ownership map was right and the
|
|
# migration simply ignored it.
|
|
# ---------------------------------------------------------------------------
|
|
|
|
CORE_MODELS_DIR = Path(__file__).resolve().parent.parent / 'shopdb' / 'core' / 'models'
|
|
|
|
# Alembic op name -> position of the table name in its positional arguments.
|
|
# create_index/drop_index take the INDEX name first and the table second, and
|
|
# every constraint op takes the CONSTRAINT name first, which is the whole
|
|
# reason this is a map and not "args[0]".
|
|
#
|
|
# The constraint ops are here because they are DDL on somebody's table just as
|
|
# much as add_column is: create_foreign_key / drop_constraint /
|
|
# create_unique_constraint / create_check_constraint / create_primary_key can
|
|
# all add or drop a constraint on core `assets` from inside a plugin chain.
|
|
# Leaving them out let that pass silently.
|
|
#
|
|
# create_foreign_key's REFERENT table is deliberately not recorded. The DDL
|
|
# runs against the source table only; naming another lane's table as a foreign
|
|
# key target is a read of its shape, not a write to it, and plugins legitimately
|
|
# point at core parents (network 0002 references `models`).
|
|
_TABLE_ARG_POSITION = {
|
|
'create_table': 0,
|
|
'drop_table': 0,
|
|
'add_column': 0,
|
|
'drop_column': 0,
|
|
'alter_column': 0,
|
|
'rename_table': 0,
|
|
'create_index': 1,
|
|
'drop_index': 1,
|
|
'create_foreign_key': 1,
|
|
'drop_constraint': 1,
|
|
'create_unique_constraint': 1,
|
|
'create_check_constraint': 1,
|
|
'create_primary_key': 1,
|
|
}
|
|
|
|
# Keywords that carry the table when it is not passed positionally. 0023 uses
|
|
# `table_name='assets'`, which an args-only scan walks straight past. `source`
|
|
# and `source_table` are the two spellings Alembic has used for
|
|
# create_foreign_key's own table; `referent`/`referent_table` are absent on
|
|
# purpose, for the reason given above.
|
|
_TABLE_KEYWORDS = ('table_name', 'source', 'source_table', 'old_table_name')
|
|
|
|
# Op name -> position of the COLUMN name in its positional arguments, for the
|
|
# ops that name exactly one column. The allowlist keys on this as well as the
|
|
# table, so an entry excusing a shipped drop_column cannot also excuse a NEW
|
|
# drop_column on the same table in the same file.
|
|
_COLUMN_ARG_POSITION = {
|
|
'add_column': 1,
|
|
'drop_column': 1,
|
|
'alter_column': 1,
|
|
}
|
|
|
|
# Same, for the batch_alter_table forms, where the table moved onto the context
|
|
# manager and every positional argument shifted left by one.
|
|
_BATCH_COLUMN_ARG_POSITION = {
|
|
'add_column': 0,
|
|
'drop_column': 0,
|
|
'alter_column': 0,
|
|
}
|
|
|
|
# --- raw SQL -----------------------------------------------------------------
|
|
# op.execute() and Connection.exec_driver_sql() hand a string straight to the
|
|
# database, so an AST scan that only knows the op.* helpers sees zero ops for
|
|
# `op.execute('ALTER TABLE assets DROP COLUMN assetnumber')`. That is the
|
|
# cheapest possible way around every other guard in this file, so the statement
|
|
# text is parsed too.
|
|
#
|
|
# Only DDL against a named table counts. UPDATE/DELETE/INSERT/SELECT are data,
|
|
# not schema: three plugin revisions already run cross-checked DML
|
|
# (backups 0002, backups 0003, computers 0006) and none of them is a lane
|
|
# crossing.
|
|
#
|
|
# Unresolvable text is rendered as a sentinel character rather than dropped, so
|
|
# the difference between "interpolates the constraint name" and "interpolates
|
|
# the table name" survives into the regex. machines 0002 does the first
|
|
# (`f"ALTER TABLE equipment DROP FOREIGN KEY {name}"` - the table is literal
|
|
# text, the FK name is not) and stays resolvable; the dodge this guards against
|
|
# is the second (`f'ALTER TABLE {t} DROP COLUMN x'`), where the sentinel lands
|
|
# inside the captured table name and the op reports UNRESOLVED, which
|
|
# test_plugin_migration_ops_all_resolve_to_a_table treats as a failure.
|
|
_SQL_HOLE = '\x00'
|
|
|
|
# Identifier characters plus the sentinel, so an interpolated name is captured
|
|
# rather than silently truncated to the literal prefix beside it.
|
|
_SQL_IDENT = r'[A-Za-z0-9_$`"\[\]\.' + _SQL_HOLE + r']+'
|
|
|
|
# (op name, compiled pattern, number of table groups). Ordered only for
|
|
# readability; every pattern is tried against every statement.
|
|
_SQL_DDL_PATTERNS = (
|
|
('execute:alter_table',
|
|
re.compile(r'\bALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?(' + _SQL_IDENT + r')', re.I)),
|
|
('execute:drop_table',
|
|
re.compile(r'\bDROP\s+TABLE\s+(?:IF\s+EXISTS\s+)?(' + _SQL_IDENT + r')', re.I)),
|
|
('execute:create_table',
|
|
re.compile(r'\bCREATE\s+(?:TEMPORARY\s+)?TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?('
|
|
+ _SQL_IDENT + r')', re.I)),
|
|
('execute:rename_table',
|
|
re.compile(r'\bRENAME\s+TABLE\s+(' + _SQL_IDENT + r')\s+TO\s+('
|
|
+ _SQL_IDENT + r')', re.I)),
|
|
('execute:create_index',
|
|
re.compile(r'\bCREATE\s+(?:UNIQUE\s+|FULLTEXT\s+|SPATIAL\s+)?INDEX\s+'
|
|
+ _SQL_IDENT + r'\s+ON\s+(' + _SQL_IDENT + r')', re.I)),
|
|
('execute:drop_index',
|
|
re.compile(r'\bDROP\s+INDEX\s+' + _SQL_IDENT + r'\s+ON\s+('
|
|
+ _SQL_IDENT + r')', re.I)),
|
|
('execute:truncate_table',
|
|
re.compile(r'\bTRUNCATE\s+(?:TABLE\s+)?(' + _SQL_IDENT + r')', re.I)),
|
|
)
|
|
|
|
# Statement verbs that are data, not schema. A statement starting with one of
|
|
# these is skipped outright, even if its body happens to contain a word like
|
|
# "table"; anything starting with something unreadable is recorded UNRESOLVED,
|
|
# because `op.execute(sql)` with sql built elsewhere is the same dodge as an
|
|
# f-string.
|
|
_SQL_DML_VERBS = ('select', 'insert', 'update', 'delete', 'replace', 'with',
|
|
'set', 'commit', 'rollback', 'analyze', 'optimize', 'call',
|
|
'pragma', 'flush', 'use', 'show', 'start')
|
|
|
|
# Historical lane crossings that have already shipped to production. Every entry
|
|
# is closed - listed so the guard is green today and red for anything new, NOT
|
|
# as a precedent to extend. Adding a line here means a plugin chain wrote to
|
|
# somebody else's table, which needs an ADR amendment, not an allowlist edit.
|
|
#
|
|
# The key is (plugin, filename, op, table, column). The column is part of it on
|
|
# purpose: keyed on the first four alone, the endpoints 0023 drop_column entry
|
|
# would also excuse a brand new `op.drop_column('assets', 'assetnumber')`
|
|
# appended to that same shipped file, which is exactly the breach this guard
|
|
# exists to stop. `None` means the op names no single column (drop_index) or
|
|
# names one the scan cannot resolve.
|
|
#
|
|
# endpoints 0023 is the CLEANUP of the 0021/0022 client-certificate breach: it
|
|
# drops the five cert columns and any index over them from the core assets
|
|
# table. It is applied on WJ production, so it cannot be edited - the fix is
|
|
# that no 0025 may do the same thing.
|
|
#
|
|
# 0021 and 0022 themselves are NOT listed, and must not be: they were rewritten
|
|
# to stamp-only no-ops, so they present no DDL to scan. Their damage lives on
|
|
# only as the 0023 cleanup below.
|
|
#
|
|
# The drop_index half of that cleanup (0023:71-73) reads alarmingly: it drops
|
|
# ANY index on assets whose column set INTERSECTS the cert columns, and never
|
|
# recreates one. An index over (assetid, client_cert_fingerprint) is dead once
|
|
# the column goes, but an index that merely happened to include a cert column
|
|
# alongside columns that still matter would be silently lost. Settled here so
|
|
# the next reader does not re-derive it - only these things ever create an index
|
|
# on assets:
|
|
# * core 68b3947ae14f baseline: idx_asset_active, idx_asset_location,
|
|
# idx_asset_status, idx_asset_type_bu, ix_assets_assetnumber,
|
|
# ix_assets_serialnumber
|
|
# * core 7b02: ix_assets_gaugelabreference, ix_assets_maintenancereference
|
|
# * core 7d33: idx_assets_levelid
|
|
# * the shipped endpoints 0021: idx_assets_cert_fingerprint over
|
|
# (client_cert_fingerprint) alone. The shipped 0022 added two more cert
|
|
# columns and no index at all.
|
|
# * db.create_all off shopdb/core/models/asset.py. Between 18e5fdc00
|
|
# (2026-09-14) and 7480e4fe3 (2026-09-15) that model DID carry the five cert
|
|
# columns, with client_cert_fingerprint declared index=True, so create_all
|
|
# built ix_assets_client_cert_fingerprint on every database created in that
|
|
# window. That is a second cert index, not just the one 0021 named, and the
|
|
# inventory above is wrong without it.
|
|
# It changes nothing for this loop. ix_assets_client_cert_fingerprint is
|
|
# single-column over client_cert_fingerprint, which 0023 drops in the very next
|
|
# statement, so the index carries no column that outlives it and there is
|
|
# nothing to recreate. The conclusion holds: the only indexes the loop can drop
|
|
# are idx_assets_cert_fingerprint and ix_assets_client_cert_fingerprint, both
|
|
# dead by construction. No index was lost and no repair migration is owed.
|
|
#
|
|
# The filter is also unreachable by accident: every name in _CERT_COLUMNS is
|
|
# underscore-spelled, which this repo's column convention forbids, so no real
|
|
# assets column can collide with it.
|
|
ALLOWED_CROSS_LANE_OPS = {
|
|
('endpoints', '0023_endpoint_credentials.py', 'drop_index', 'assets', None),
|
|
('endpoints', '0023_endpoint_credentials.py', 'drop_column', 'assets',
|
|
'client_cert_fingerprint'),
|
|
('endpoints', '0023_endpoint_credentials.py', 'drop_column', 'assets',
|
|
'cert_issued_at'),
|
|
('endpoints', '0023_endpoint_credentials.py', 'drop_column', 'assets',
|
|
'cert_expires_at'),
|
|
('endpoints', '0023_endpoint_credentials.py', 'drop_column', 'assets',
|
|
'cert_revoked_at'),
|
|
('endpoints', '0023_endpoint_credentials.py', 'drop_column', 'assets',
|
|
'cert_revocation_reason'),
|
|
}
|
|
|
|
|
|
def _core_tablenames() -> set:
|
|
"""Every table the CORE chain owns, read statically off shopdb/core/models.
|
|
|
|
Picks up both `__tablename__` on a model and the bare `db.Table(...)`
|
|
association tables (userroles, rolepermissions, userpermissions), which have
|
|
no model class and would otherwise read as unowned.
|
|
"""
|
|
names = set()
|
|
for source in CORE_MODELS_DIR.glob('*.py'):
|
|
tree = ast.parse(source.read_text())
|
|
for node in ast.walk(tree):
|
|
if isinstance(node, ast.Assign):
|
|
targets = [t.id for t in node.targets if isinstance(t, ast.Name)]
|
|
if '__tablename__' in targets and isinstance(node.value, ast.Constant):
|
|
names.add(node.value.value)
|
|
if isinstance(node, ast.Call):
|
|
func = node.func
|
|
name = func.attr if isinstance(func, ast.Attribute) else getattr(func, 'id', None)
|
|
if name == 'Table' and node.args:
|
|
first = node.args[0]
|
|
if isinstance(first, ast.Constant) and isinstance(first.value, str):
|
|
names.add(first.value)
|
|
return names
|
|
|
|
|
|
def _table_owner_map() -> dict:
|
|
"""table name -> owning lane ('core' or a plugin name)."""
|
|
owners = {}
|
|
for plugin, tables in PLUGIN_TABLE_OWNERS.items():
|
|
for tablename in tables:
|
|
owners[tablename] = plugin
|
|
for tablename in _core_tablenames():
|
|
# A plugin entry wins: pre-cutover the core chain CREATED the plugin
|
|
# tables, so a name can look core-shaped and still belong to a plugin.
|
|
owners.setdefault(tablename, 'core')
|
|
return owners
|
|
|
|
|
|
def _module_constants(tree) -> dict:
|
|
"""Module-level `NAME = 'sometable'` bindings.
|
|
|
|
Half the chains name their tables through constants (CREDENTIALS,
|
|
REQUESTS, _APPS), so a scan that only understands string literals sees
|
|
almost nothing.
|
|
"""
|
|
consts = {}
|
|
for node in tree.body:
|
|
if isinstance(node, ast.Assign) and isinstance(node.value, ast.Constant) \
|
|
and isinstance(node.value.value, str):
|
|
for target in node.targets:
|
|
if isinstance(target, ast.Name):
|
|
consts[target.id] = node.value.value
|
|
return consts
|
|
|
|
|
|
def _module_sequences(tree) -> dict:
|
|
"""Module-level `NAME = ('a', 'b')` bindings of all-string sequences.
|
|
|
|
Needed for the column half of the allowlist key: endpoints 0023 drops its
|
|
five columns with `for column in _CERT_COLUMNS`, and without this the column
|
|
resolves to nothing and the allowlist entry degrades back to a blanket
|
|
"drop_column on assets from this file is fine".
|
|
"""
|
|
seqs = {}
|
|
for node in tree.body:
|
|
if not isinstance(node, ast.Assign):
|
|
continue
|
|
if not isinstance(node.value, (ast.Tuple, ast.List, ast.Set)):
|
|
continue
|
|
values = set()
|
|
for element in node.value.elts:
|
|
if not (isinstance(element, ast.Constant) and isinstance(element.value, str)):
|
|
values = set()
|
|
break
|
|
values.add(element.value)
|
|
if not values:
|
|
continue
|
|
for target in node.targets:
|
|
if isinstance(target, ast.Name):
|
|
seqs[target.id] = frozenset(values)
|
|
return seqs
|
|
|
|
|
|
def _literal(node, consts):
|
|
if isinstance(node, ast.Constant) and isinstance(node.value, str):
|
|
return node.value
|
|
if isinstance(node, ast.Name):
|
|
return consts.get(node.id)
|
|
return None
|
|
|
|
|
|
def _loop_bindings(tree, consts, seqs) -> dict:
|
|
"""Map each Call node to the loop variables in scope over a literal sequence.
|
|
|
|
`for table in (IMPACTS, TRANSITIONS, HEALTHSTATES): op.drop_table(table)`
|
|
(endpoints 0017) and the same shape in guides 0001 are real DDL over real
|
|
tables. Without this they resolve to nothing, and "resolves to nothing" has
|
|
to be a failure or the guard is dodgeable by writing a loop.
|
|
|
|
A bare module constant as the iterable (`for column in _CERT_COLUMNS`) is
|
|
resolved through `seqs` for the same reason.
|
|
"""
|
|
bindings = {}
|
|
for node in ast.walk(tree):
|
|
if not isinstance(node, ast.For) or not isinstance(node.target, ast.Name):
|
|
continue
|
|
values = set()
|
|
if isinstance(node.iter, (ast.Tuple, ast.List, ast.Set)):
|
|
for element in node.iter.elts:
|
|
value = _literal(element, consts)
|
|
if value is None:
|
|
values = set()
|
|
break
|
|
values.add(value)
|
|
elif isinstance(node.iter, ast.Name):
|
|
values = set(seqs.get(node.iter.id, ()))
|
|
if not values:
|
|
continue
|
|
# ast.walk is breadth-first from the top, so an inner loop rebinding the
|
|
# same name is visited after the outer one and correctly shadows it.
|
|
for sub in ast.walk(node):
|
|
if isinstance(sub, ast.Call):
|
|
bindings.setdefault(id(sub), {})[node.target.id] = frozenset(values)
|
|
return bindings
|
|
|
|
|
|
def _resolve_tables(node, consts, loopvars) -> frozenset:
|
|
"""Every table name an argument could be. Empty means unresolvable."""
|
|
if isinstance(node, ast.Constant) and isinstance(node.value, str):
|
|
return frozenset({node.value})
|
|
if isinstance(node, ast.Name):
|
|
if node.id in loopvars:
|
|
return loopvars[node.id]
|
|
if node.id in consts:
|
|
return frozenset({consts[node.id]})
|
|
return frozenset()
|
|
|
|
|
|
def _resolve_columns(opname, node, args, consts, loopvars, positions) -> frozenset:
|
|
"""The column name(s) an op names, or empty when it names none.
|
|
|
|
Empty is NOT a failure here the way it is for a table: most ops name no
|
|
column at all. It only widens the allowlist key to None.
|
|
"""
|
|
position = positions.get(opname)
|
|
if position is None or len(args) <= position:
|
|
return frozenset()
|
|
argument = args[position]
|
|
if opname == 'add_column':
|
|
# add_column takes sa.Column('name', ...), not a bare string.
|
|
if isinstance(argument, ast.Call) and argument.args:
|
|
argument = argument.args[0]
|
|
else:
|
|
return frozenset()
|
|
return _resolve_tables(argument, consts, loopvars)
|
|
|
|
|
|
def _flatten_sql(node, consts) -> str:
|
|
"""Statement text with every unresolvable fragment replaced by a sentinel.
|
|
|
|
Keeping the hole in place (instead of giving up on the whole string) is what
|
|
lets `f"ALTER TABLE equipment DROP FOREIGN KEY {name}"` resolve to
|
|
`equipment` while `f'ALTER TABLE {t} DROP COLUMN x'` resolves to nothing.
|
|
"""
|
|
if isinstance(node, ast.Constant):
|
|
return node.value if isinstance(node.value, str) else _SQL_HOLE
|
|
if isinstance(node, ast.JoinedStr):
|
|
return ''.join(_flatten_sql(value, consts) for value in node.values)
|
|
if isinstance(node, ast.FormattedValue):
|
|
return _SQL_HOLE
|
|
if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add):
|
|
return _flatten_sql(node.left, consts) + _flatten_sql(node.right, consts)
|
|
if isinstance(node, ast.Name):
|
|
return consts.get(node.id, _SQL_HOLE)
|
|
if isinstance(node, ast.Call):
|
|
# sa.text('...') / text('...') is a wrapper, not a hole: computers 0006
|
|
# passes its statement that way.
|
|
func = node.func
|
|
name = func.attr if isinstance(func, ast.Attribute) else getattr(func, 'id', None)
|
|
if name == 'text' and len(node.args) == 1:
|
|
return _flatten_sql(node.args[0], consts)
|
|
return _SQL_HOLE
|
|
|
|
|
|
def _clean_sql_identifier(raw: str) -> str:
|
|
"""`db`.`assets` -> assets. Keeps the sentinel if one landed in the name."""
|
|
name = raw.strip().rstrip('(;,')
|
|
if '.' in name:
|
|
name = name.rsplit('.', 1)[-1]
|
|
return name.strip('`"[]')
|
|
|
|
|
|
def _scan_raw_sql(node, consts) -> list:
|
|
"""(opname, frozenset-of-tables) for the DDL in one execute-style call.
|
|
|
|
An empty table set is UNRESOLVED and the caller fails on it. A statement
|
|
that is plainly DML returns nothing at all.
|
|
"""
|
|
statement = _flatten_sql(node, consts)
|
|
found = []
|
|
# Classify PER STATEMENT, not per call. Reading the first verb and bailing
|
|
# let a leading DML verb hide trailing DDL: op.execute("SET @x = 1; ALTER
|
|
# TABLE assets DROP COLUMN serialnumber") was skipped whole, because the
|
|
# scan saw SET, called it DML and returned nothing.
|
|
for part in statement.split(';'):
|
|
stripped = part.lstrip().lstrip('(')
|
|
verb = stripped.split(None, 1)[0].lower() if stripped.split() else ''
|
|
if not verb:
|
|
continue
|
|
if verb.startswith(_SQL_HOLE):
|
|
# The verb itself is interpolated, so nothing about this statement
|
|
# can be checked. Unresolved op rather than no op.
|
|
found.append(('execute:unknown', frozenset()))
|
|
continue
|
|
if verb in _SQL_DML_VERBS:
|
|
continue
|
|
for opname, pattern in _SQL_DDL_PATTERNS:
|
|
for match in pattern.finditer(part):
|
|
for group in match.groups():
|
|
name = _clean_sql_identifier(group)
|
|
if not name or _SQL_HOLE in name:
|
|
found.append((opname, frozenset()))
|
|
else:
|
|
found.append((opname, frozenset({name})))
|
|
return found
|
|
|
|
|
|
def _scan_plugin_migration_ops() -> list:
|
|
"""Every schema op in every plugin chain, as
|
|
(plugin, filename, lineno, opname, frozenset-of-tables,
|
|
frozenset-of-columns).
|
|
|
|
An empty table set means the argument could not be resolved statically;
|
|
the caller treats that as a failure rather than a pass. An empty column set
|
|
only means the op names no resolvable single column.
|
|
"""
|
|
found = []
|
|
for script in sorted(PLUGINS_DIR.glob('*/migrations/versions/*.py')):
|
|
plugin = script.parts[-4]
|
|
tree = ast.parse(script.read_text())
|
|
consts = _module_constants(tree)
|
|
seqs = _module_sequences(tree)
|
|
loops = _loop_bindings(tree, consts, seqs)
|
|
|
|
# batch_alter_table('assets') moves the table onto the context manager,
|
|
# so batch_op.drop_column('x') names a column where op.drop_column names
|
|
# a table. Resolve those against the `with` instead.
|
|
batched = {}
|
|
for node in ast.walk(tree):
|
|
if not isinstance(node, ast.With):
|
|
continue
|
|
for item in node.items:
|
|
call = item.context_expr
|
|
if not (isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute)
|
|
and call.func.attr == 'batch_alter_table' and call.args):
|
|
continue
|
|
tables = _resolve_tables(call.args[0], consts, loops.get(id(call), {}))
|
|
alias = item.optional_vars.id if isinstance(item.optional_vars, ast.Name) else None
|
|
for sub in ast.walk(node):
|
|
if isinstance(sub, ast.Call) and isinstance(sub.func, ast.Attribute) \
|
|
and sub.func.attr in _TABLE_ARG_POSITION \
|
|
and isinstance(sub.func.value, ast.Name) \
|
|
and sub.func.value.id == alias:
|
|
batched[id(sub)] = tables
|
|
|
|
for node in ast.walk(tree):
|
|
if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)):
|
|
continue
|
|
opname = node.func.attr
|
|
base = node.func.value
|
|
|
|
# Raw SQL: op.execute(...), bind.execute(...) and any
|
|
# <connection>.exec_driver_sql(...).
|
|
#
|
|
# Both names are matched on the method name ALONE, without looking at
|
|
# what they are called on. Requiring the receiver to be literally `op`
|
|
# missed the repo's DOMINANT idiom: 24 call sites across endpoints
|
|
# 0006/0007/0010/0019, network 0003 and notifications 0005 all do
|
|
# `bind = op.get_bind()` and then `bind.execute(...)`, so a plugin
|
|
# could drop a column off the core assets table through a bind and
|
|
# the scan recorded nothing at all. Every shipped bind.execute passes
|
|
# a literal SELECT or UPDATE, which _SQL_DML_VERBS skips, so widening
|
|
# this costs no false positives.
|
|
if opname in ('exec_driver_sql', 'execute'):
|
|
if not node.args:
|
|
continue
|
|
for sqlop, tables in _scan_raw_sql(node.args[0], consts):
|
|
found.append((plugin, script.name, node.lineno, sqlop,
|
|
tables, frozenset()))
|
|
continue
|
|
|
|
if opname not in _TABLE_ARG_POSITION:
|
|
continue
|
|
loopvars = loops.get(id(node), {})
|
|
if id(node) in batched:
|
|
columns = _resolve_columns(opname, node, node.args, consts, loopvars,
|
|
_BATCH_COLUMN_ARG_POSITION)
|
|
found.append((plugin, script.name, node.lineno,
|
|
'batch_' + opname, batched[id(node)], columns))
|
|
continue
|
|
if not (isinstance(base, ast.Name) and base.id == 'op'):
|
|
continue
|
|
position = _TABLE_ARG_POSITION[opname]
|
|
tables = frozenset()
|
|
if len(node.args) > position:
|
|
tables = _resolve_tables(node.args[position], consts, loopvars)
|
|
for keyword in node.keywords:
|
|
if keyword.arg in _TABLE_KEYWORDS:
|
|
tables = tables | _resolve_tables(keyword.value, consts, loopvars)
|
|
columns = _resolve_columns(opname, node, node.args, consts, loopvars,
|
|
_COLUMN_ARG_POSITION)
|
|
found.append((plugin, script.name, node.lineno, opname, tables, columns))
|
|
if opname == 'rename_table' and len(node.args) > 1:
|
|
# Both ends of a rename are DDL on a table.
|
|
found.append((plugin, script.name, node.lineno, 'rename_table',
|
|
_resolve_tables(node.args[1], consts, loopvars),
|
|
frozenset()))
|
|
return found
|
|
|
|
|
|
def _cross_lane_ops() -> list:
|
|
"""Scanned ops whose table belongs to core or to a different plugin.
|
|
|
|
A table that is in neither the core models nor PLUGIN_TABLE_OWNERS is NOT a
|
|
crossing: that is a pre-rename name (machines 0002 renames equipment ->
|
|
machines) or a table since dropped, and neither reaches into another lane.
|
|
Ownership drift of a table that DOES still exist is already covered by
|
|
test_owned_tables_match_declared_models.
|
|
"""
|
|
owners = _table_owner_map()
|
|
crossings = []
|
|
for plugin, filename, lineno, opname, tables, columns in _scan_plugin_migration_ops():
|
|
for tablename in sorted(tables):
|
|
owner = owners.get(tablename)
|
|
if owner is None or owner == plugin:
|
|
continue
|
|
for column in (sorted(columns) or [None]):
|
|
crossings.append((plugin, filename, lineno, opname, tablename,
|
|
owner, column))
|
|
return crossings
|
|
|
|
|
|
def _crossing_keys() -> set:
|
|
return {(plugin, filename, opname, tablename, column)
|
|
for plugin, filename, _, opname, tablename, _, column in _cross_lane_ops()}
|
|
|
|
|
|
def test_plugin_migration_ops_all_resolve_to_a_table():
|
|
"""Every scanned op names a table the scan can actually resolve.
|
|
|
|
Without this the guard is dodgeable by accident: a table name built at
|
|
runtime resolves to nothing, and "nothing" would otherwise read as "touches
|
|
no foreign table" and pass. It is also what makes the raw-SQL scan real -
|
|
`op.execute(f'ALTER TABLE {t} ...')` lands here rather than going quietly
|
|
green.
|
|
"""
|
|
unresolved = [(plugin, filename, lineno, opname)
|
|
for plugin, filename, lineno, opname, tables, _
|
|
in _scan_plugin_migration_ops() if not tables]
|
|
assert not unresolved, (
|
|
'plugin migration ops whose table name could not be resolved '
|
|
f'statically: {unresolved}')
|
|
|
|
|
|
def test_scan_sees_the_known_lane_crossing():
|
|
"""The scanner is not vacuous: it still finds the 0023 assets cleanup.
|
|
|
|
If a refactor breaks the AST walk, every other assertion here goes green on
|
|
an empty result set. This one goes red.
|
|
"""
|
|
scanned = _scan_plugin_migration_ops()
|
|
assert len(scanned) > 100, f'scan found only {len(scanned)} ops; it is broken'
|
|
keys = _crossing_keys()
|
|
assert ALLOWED_CROSS_LANE_OPS <= keys, (
|
|
'the allowlisted historical crossings are no longer detected: '
|
|
f'{ALLOWED_CROSS_LANE_OPS - keys}')
|
|
|
|
|
|
def test_scan_reads_raw_sql_ddl():
|
|
"""The raw-SQL half is not vacuous either.
|
|
|
|
machines 0002 does its whole MySQL rename through exec_driver_sql. If the
|
|
statement parser stops working, those ops vanish and nothing else in this
|
|
file notices.
|
|
"""
|
|
raw = [(plugin, filename, opname, tuple(sorted(tables)))
|
|
for plugin, filename, _, opname, tables, _
|
|
in _scan_plugin_migration_ops() if opname.startswith('execute:')]
|
|
assert ('machines', '0002_rename_from_equipment.py', 'execute:rename_table',
|
|
('equipmenttypes',)) in raw, raw
|
|
assert ('machines', '0002_rename_from_equipment.py', 'execute:alter_table',
|
|
('machines',)) in raw, raw
|
|
# DML stays out of it, or the three shipped UPDATE statements would read as
|
|
# schema changes on tables their own plugin owns.
|
|
assert not [entry for entry in raw
|
|
if entry[0] == 'backups' or entry[1].startswith('0006_policy')], raw
|
|
|
|
|
|
def test_no_plugin_migration_touches_another_lanes_table():
|
|
"""ADR-008: a plugin chain does DDL only on the tables it owns.
|
|
|
|
Fails on create_table/drop_table/add_column/drop_column/alter_column/
|
|
create_index/drop_index/rename_table, the five constraint ops, the
|
|
batch_alter_table forms, and DDL inside op.execute/exec_driver_sql, against
|
|
a core table or another plugin's table. The allowlist above holds exactly
|
|
the entries that already shipped.
|
|
"""
|
|
offenders = [crossing for crossing in _cross_lane_ops()
|
|
if (crossing[0], crossing[1], crossing[3], crossing[4], crossing[6])
|
|
not in ALLOWED_CROSS_LANE_OPS]
|
|
assert not offenders, '\n'.join(
|
|
f'{plugin} chain: {filename}:{lineno} {opname} on {tablename!r}'
|
|
+ (f' column {column!r}' if column else '')
|
|
+ f', owned by {owner} (ADR-008)'
|
|
for plugin, filename, lineno, opname, tablename, owner, column in offenders)
|
|
|
|
|
|
def test_cross_lane_allowlist_has_no_dead_entries():
|
|
"""The allowlist does not outlive what it excuses.
|
|
|
|
An entry left behind after its migration is squashed or rewritten is a
|
|
standing licence to write to a core table from that file again.
|
|
"""
|
|
keys = _crossing_keys()
|
|
assert not (ALLOWED_CROSS_LANE_OPS - keys), (
|
|
'allowlist entries that match nothing in the tree: '
|
|
f'{ALLOWED_CROSS_LANE_OPS - keys}')
|