Stop the collector being able to set a printer assignment
Some checks failed
CI / backend (push) Failing after 11s
CI / naming (push) Successful in 9s
CI / frontend (push) Successful in 10s
CI / migrations-mysql (push) Failing after 7s

The printer feature rests on one rule: a collector report is OBSERVED state and
never ASSIGNED state. If a drifted bay's own state can become what it is told to
install, enforcement stops meaning anything, because a bay that installed the
wrong printer makes itself right simply by reporting it.

_sync_printer_links broke that rule. It ran on every collector PC push and, when
the payload carried defaultprinter or printers, wrote defaultprinter and
connectedto rows from the report - defaultprinter being one of the two types
_reconcile_edges is meant to own alone, from authenticated routes a human calls.

It was recorded as dormant. It was not. The collector schema ADVERTISED both
keys and the example client in COLLECTOR-INTEGRATION.md set
$payload['defaultprinter'], so the documentation was recruiting callers for it -
while the same document, under "Observed is not assigned", promised that nothing
on this path writes a usesprinter or defaultprinter row. The document
contradicted itself and the code followed the wrong half. Nothing in the fleet
armed it, because the first-party reporter never sent those fields, but any
third-party client written from the docs would have.

Retired the function, its _sync_one helper and the now-unused
PRINTER_LINK_ORIGIN label, and removed both keys from the schema.

IGNORED, NOT REJECTED. Unknown payload keys are not validated, so a client
written against the old docs keeps pushing successfully and only its printer
keys stop doing anything. The response warns, naming the keys and pointing at
POST /api/collector/printers for reporting queues and seed-from-observed for
adopting them. Dropping them in silence would leave an integrator believing
assignments were still being written, which is a worse failure than the one
being fixed.

The docs were the larger half of this. The schema table rows are gone, the
"PC -> printer relationship sync" section is replaced by a note saying what
happens now to anyone still sending the keys, and the example client no longer
enumerates Win32_Printer at all - it points at Report-PrintersToShopDB.ps1,
which posts to the observed endpoint and posts NOTHING on a failed enumeration,
since an empty list would erase the host's rows.

test_collector_printers.py pinned the old behaviour, so it now pins the
opposite: the keys create no relationships, the push still succeeds and says
why, a payload without them stays quiet, a human-made default survives a report
naming a different printer, and the schema no longer offers the keys.
This commit is contained in:
cproudlock
2026-08-24 11:25:02 -04:00
parent 24435e6359
commit adc7ef43f9
5 changed files with 148 additions and 312 deletions

View File

@@ -1,11 +1,19 @@
"""Tests for the computers collector PC->printer relationship sync.
"""The computers collector must NEVER write a printer assignment.
A collector payload carrying defaultprinter/printers creates AssetRelationship
rows linking the PC asset to the resolved printer assets ('defaultprinter' for
the default, 'connectedto' for the rest). Stale collector links archive on
re-report; manual links survive; unresolved identifiers warn without failing.
This file used to pin the opposite: `_sync_printer_links` turned a payload's
`defaultprinter`/`printers` into AssetRelationship rows. That contradicted the
rule the printer feature rests on - ASSIGNED and OBSERVED never mix - because a
bay that installed the wrong printer would have made itself right just by
reporting it.
It was not dormant. The collector schema advertised both keys and
COLLECTOR-INTEGRATION.md's example client set `defaultprinter`, while another
page of the same document promised nothing on this path writes an assignment
row. Retired 2026-08-24; these tests are the guard that it stays retired.
Observed queues have their own path (POST /api/collector/printers) and become
assignments only when a person asks, via seed-from-observed.
"""
import pytest
from shopdb.core.models import (
@@ -81,119 +89,75 @@ def _active_links(db, pcid):
sourceassetid=pcid, isactive=True).all()
def test_creates_default_and_connected_links(client, db, collector_key, scene):
"""Default -> defaultprinter, others -> connectedto, resolved by three id forms."""
resp = _post(client, {
'hostname': 'WJPC-P1',
'defaultprinter': 'HP-Materials', # windowsname
'printers': ['prn-front.wjs.local', '10.1.2.3'], # hostname, IP
})
assert resp.status_code == 200, resp.get_json()
data = resp.get_json()['data']
assert data['printerlinkcount'] == 3
assert not data['warnings']
pc = _pc_asset(db)
links = {(r.targetassetid, r.relationshiptype.relationshiptype)
for r in _active_links(db, pc.assetid)}
assert (scene['a'].assetid, 'defaultprinter') in links
assert (scene['b'].assetid, 'connectedto') in links
assert (scene['c'].assetid, 'connectedto') in links
def test_unresolved_identifier_warns_not_fails(client, db, collector_key, scene):
"""An unknown printer identifier is a warning, the rest still link."""
resp = _post(client, {
'hostname': 'WJPC-P2',
def test_printer_keys_create_no_relationships(client, db, collector_key, scene):
"""The reported case, inverted: sending them must change nothing."""
response = _post(client, {
'hostname': 'BAY-3105-PC',
'defaultprinter': 'HP-Materials',
'printers': ['does-not-exist'],
'printers': ['HP-Materials', 'prn-front.wjs.local', '10.1.2.3'],
})
assert resp.status_code == 200
data = resp.get_json()['data']
assert any('unresolved printer: does-not-exist' in w
for w in data['warnings'])
pc = _pc_asset(db)
assert data['printerlinkcount'] == 1
assert len(_active_links(db, pc.assetid)) == 1
assert response.status_code in (200, 201), response.get_json()
pcid = _pc_asset(db).assetid
assert _active_links(db, pcid) == [], (
'a collector report wrote printer relationships')
def test_idempotent_no_duplicates(client, db, collector_key, scene):
"""Re-report the same set: no new rows, still one link per printer."""
payload = {'hostname': 'WJPC-P3', 'defaultprinter': 'HP-Materials',
'printers': ['Zebra-Ship']}
assert _post(client, payload).status_code == 200
assert _post(client, payload).status_code == 200
def test_the_push_still_succeeds_and_says_why(client, db, collector_key, scene):
"""Warn, do not reject.
pc = _pc_asset(db)
all_rows = AssetRelationship.query.filter_by(sourceassetid=pc.assetid).all()
assert len(all_rows) == 2 # one default + one connected, no dupes
assert all(r.isactive for r in all_rows)
Unknown keys are not validated, so a client written against the old docs
keeps pushing successfully - only its printer keys stop doing anything.
Silently dropping them would leave an integrator believing assignments were
still being written, which is worse than the original bug.
"""
response = _post(client, {
'hostname': 'BAY-3105-PC',
'defaultprinter': 'HP-Materials',
})
assert response.status_code in (200, 201)
warnings = ' '.join(response.get_json()['data']['warnings']).lower()
assert 'defaultprinter' in warnings
assert 'seed-from-observed' in warnings, (
'the warning should say where assignment actually comes from')
def test_dropped_printer_is_archived(client, db, collector_key, scene):
"""A printer that stops being reported loses its collector link."""
_post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials',
'printers': ['Zebra-Ship']})
pc = _pc_asset(db)
assert len(_active_links(db, pc.assetid)) == 2
# Re-report without Zebra-Ship.
_post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials',
'printers': []})
active = _active_links(db, pc.assetid)
assert len(active) == 1
assert active[0].targetassetid == scene['a'].assetid
# the dropped row still exists but is inactive
zebra = AssetRelationship.query.filter_by(
sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first()
assert zebra is not None and zebra.isactive is False
def test_a_payload_without_printer_keys_warns_about_nothing(
client, db, collector_key, scene):
"""The normal case must stay quiet - this is every push in the fleet."""
response = _post(client, {'hostname': 'BAY-3105-PC'})
assert response.status_code in (200, 201)
warnings = ' '.join(response.get_json()['data']['warnings']).lower()
assert 'printer' not in warnings
def test_default_change_archives_old_default(client, db, collector_key, scene):
"""Changing the default printer archives the previous defaultprinter link."""
_post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'HP-Materials'})
pc = _pc_asset(db)
_post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'prn-front.wjs.local'})
active = _active_links(db, pc.assetid)
assert len(active) == 1
assert active[0].targetassetid == scene['b'].assetid
assert active[0].relationshiptype.relationshiptype == 'defaultprinter'
def test_manual_link_survives_collector_sync(client, db, collector_key, scene):
"""A manually-created link (no collector label) is never archived."""
# PC created first via a bare report.
_post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials',
'printers': ['Zebra-Ship']})
pc = _pc_asset(db)
# Manual connectedto link PC -> printer B (label stays NULL).
def test_a_manual_assignment_survives_a_report_that_names_printers(
client, db, collector_key, scene):
"""The row a person made is the one that matters, and it stays untouched."""
_post(client, {'hostname': 'BAY-3105-PC'})
pcasset = _pc_asset(db)
manual = AssetRelationship(
sourceassetid=pc.assetid, targetassetid=scene['b'].assetid,
relationshiptypeid=scene['ct_type'].relationshiptypeid, notes='by hand')
sourceassetid=pcasset.assetid, targetassetid=scene['a'].assetid,
relationshiptypeid=scene['dp_type'].relationshiptypeid)
db.session.add(manual)
db.session.commit()
manual_id = manual.relationshipid
# Re-report dropping Zebra-Ship and never mentioning printer B.
_post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials',
'printers': []})
_post(client, {
'hostname': 'BAY-3105-PC',
'defaultprinter': 'prn-front.wjs.local', # a DIFFERENT printer
'printers': ['prn-front.wjs.local'],
})
surviving = db.session.get(AssetRelationship, manual_id)
assert surviving.isactive is True # manual link untouched
# Zebra (collector) archived.
zebra = AssetRelationship.query.filter_by(
sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first()
assert zebra.isactive is False
links = _active_links(db, pcasset.assetid)
assert len(links) == 1
assert links[0].targetassetid == scene['a'].assetid, (
'the report retargeted a human-made default printer')
def test_no_printer_keys_leaves_links_untouched(client, db, collector_key, scene):
"""A report with no printer keys does not archive existing links."""
_post(client, {'hostname': 'WJPC-P7', 'defaultprinter': 'HP-Materials'})
pc = _pc_asset(db)
assert len(_active_links(db, pc.assetid)) == 1
def test_the_retired_keys_are_gone_from_the_collector_schema(app):
"""The schema advertised them, which is how integrators found them."""
from plugins.computers.plugin import ComputersPlugin
# A plain update (no defaultprinter, no printers key) must not archive.
_post(client, {'hostname': 'WJPC-P7', 'currentuser': 'alice'})
assert len(_active_links(db, pc.assetid)) == 1
fields = ComputersPlugin().get_collector_schema()['fields']
assert 'defaultprinter' not in fields
assert 'printers' not in fields