diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 64b4011..200fc71 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -726,17 +726,49 @@ def publish_scope_route(scopeid): if not scope: return error_response(ErrorCodes.NOT_FOUND, 'No such scope', http_code=404) from flask_jwt_extended import get_jwt_identity - notes = (request.get_json(silent=True) or {}).get('notes') + body = request.get_json(silent=True) or {} + notes = body.get('notes') try: publishedby = int(get_jwt_identity()) except (TypeError, ValueError): publishedby = None - version = service.publish_scope(scope.scopename, scope.phase, - notes=notes, publishedby=publishedby) + try: + version = service.publish_scope( + scope.scopename, scope.phase, notes=notes, + publishedby=publishedby, force=bool(body.get('force'))) + except service.LibVersionTooOldError as error: + # 409, not 500: the request is well-formed, the fleet is not ready. The + # hosts come back so the caller can act without going hunting. + return error_response( + ErrorCodes.CONFLICT, str(error), http_code=409, + details={'required': error.required, 'floor': error.floor, + 'hosts': error.hosts}) db.session.commit() return success_response({'versionnumber': version}, http_code=201) +@geenforce_bp.route('/scopes//publish-preflight', methods=['GET']) +@jwt_required() +@require_permission('geenforce.manage') +def publish_preflight(scopeid): + """Would publishing this scope outrun the fleet's enforcer lib? + + Lets the UI warn BEFORE someone clicks publish, rather than only refusing + afterwards. + """ + scope = db.session.get(ManifestScope, scopeid) + if not scope: + return error_response(ErrorCodes.NOT_FOUND, 'No such scope', http_code=404) + hosts, floor = service.hosts_below_libversion( + scope.scopename, scope.phase, scope.manifestversion) + return success_response({ + 'required': scope.manifestversion, + 'floor': floor, + 'hostsbehind': hosts, + 'canpublish': not hosts, + }) + + @geenforce_bp.route('/scopes//versions', methods=['GET']) @jwt_required() @require_permission('geenforce.manage') diff --git a/plugins/geenforce/plugin.py b/plugins/geenforce/plugin.py index 24a2c6a..31a2115 100644 --- a/plugins/geenforce/plugin.py +++ b/plugins/geenforce/plugin.py @@ -218,13 +218,21 @@ class GeEnforcePlugin(BasePlugin): @click.argument('scopename') @click.option('--phase', default='runtime') @click.option('--notes', default=None) - def publish_cmd(scopename, phase, notes): + @click.option('--force', is_flag=True, + help='Publish even when reporting PCs run an older ' + 'enforcer lib than the manifest requires.') + def publish_cmd(scopename, phase, notes, force): """Freeze the current draft of a scope into a published snapshot.""" from flask import current_app - from .service import publish_scope + from .service import publish_scope, LibVersionTooOldError with current_app.app_context(): - version = publish_scope(scopename, phase, notes=notes) + try: + version = publish_scope(scopename, phase, notes=notes, + force=force) + except LibVersionTooOldError as error: + click.echo(click.style(f'REFUSED: {error}', fg='red')) + raise SystemExit(1) db.session.commit() click.echo(f"Published {scopename}/{phase} as v{version}.") diff --git a/plugins/geenforce/service.py b/plugins/geenforce/service.py index 3fe3bc1..657f39d 100644 --- a/plugins/geenforce/service.py +++ b/plugins/geenforce/service.py @@ -26,10 +26,77 @@ from .importer import build_entry from .serializer import scope_to_json +class LibVersionTooOldError(Exception): + """Publishing this scope would reach PCs whose enforcer lib is too old. + + Carries the detail a human needs to act: what the manifest requires, the + lowest lib version the fleet reports for this scope, and which hosts are + behind. + """ + + def __init__(self, scopename, required, floor, hosts): + self.scopename = scopename + self.required = required + self.floor = floor + self.hosts = hosts + shown = ', '.join(hosts[:10]) + if len(hosts) > 10: + shown += f' (+{len(hosts) - 10} more)' + super().__init__( + f'{scopename} manifest is version {required} but {len(hosts)} ' + f'reporting PC(s) run enforcer lib {floor}: {shown}. ' + 'Push the newer lib to the fleet first, or publish with force=True ' + 'if you are certain those PCs must not receive this scope.') + + def _utcnow(): return datetime.now(timezone.utc).replace(tzinfo=None) +def parse_libversion(text): + """'2.6' -> (2, 6). Returns None for missing or unparseable values. + + None means "cannot tell", and every caller treats that as "do not claim + this PC is safe" rather than "this PC is fine" - an unreadable version is + exactly the case where guessing is expensive. + """ + if not text: + return None + parts = str(text).strip().split('.') + try: + return (int(parts[0]), int(parts[1]) if len(parts) > 1 else 0) + except (ValueError, IndexError): + return None + + +def hosts_below_libversion(scopename, phase, required): + """Hosts whose CURRENT report for this scope runs a lib older than required. + + A report with no/unparseable enforcerversion counts as behind: the field was + added with the summary-emitting engine, so its absence means an old lib. + + Returns (hosts, floor) where floor is the lowest parseable version string + seen, or None when nothing reported. + """ + wanted = parse_libversion(required) + if not wanted: + return [], None + + reports = ManifestEnforcementReport.query.filter_by( + scopename=scopename, phase=phase, iscurrent=True).all() + + behind = [] + lowest = None + for report in reports: + actual = parse_libversion(report.enforcerversion) + if actual is None or actual < wanted: + behind.append(report.hostname) + if actual is not None and (lowest is None or actual < lowest): + lowest = actual + floor = f'{lowest[0]}.{lowest[1]}' if lowest else None + return sorted(set(behind)), floor + + def _installed_app_model(): """Lazily import the computers plugin's ComputerInstalledApp. @@ -72,14 +139,38 @@ def replace_scope_draft(scopename, phase, manifest): return scope -def publish_scope(scopename, phase, notes=None, publishedby=None): +def publish_scope(scopename, phase, notes=None, publishedby=None, force=False): """Freeze the current draft into a new published snapshot. Returns the - version number (uncommitted).""" + version number (uncommitted). + + Refuses when the manifest is newer than the enforcer lib on PCs that report + for this scope, unless force=True. + + WHY THIS GATE EXISTS: the lib treats a minor-newer manifest as backward + compatible and carries on. That holds for additions that WIDEN behaviour (a + new Type is skipped by an old lib), and inverts for one that NARROWS it. The + _CmmVersion filter arrived in lib 2.6 as a minor bump: an older lib does not + know the field, so every gated entry looks unfiltered and it installs EVERY + PC-DMIS version it cannot detect, on every CMM, within one cycle. The share + runbook says push the lib first; a runbook is not a control. This is, and it + reads the version the fleet actually reports rather than the one someone + believes is deployed. + + A scope nobody has reported for yet cannot be checked, so it publishes: a + fresh site would otherwise be unable to publish anything at all. + """ scope = ManifestScope.query.filter_by( scopename=scopename, phase=phase).first() if not scope: raise ValueError(f'No scope {scopename}/{phase}') + if not force: + behind, floor = hosts_below_libversion( + scopename, phase, scope.manifestversion) + if behind: + raise LibVersionTooOldError( + scopename, scope.manifestversion, floor or 'unknown', behind) + text = scope_to_json(scope) maxversion = db.session.query( func.max(ManifestPublishedVersion.versionnumber) diff --git a/tests/test_plugins/test_geenforce_publish_gate.py b/tests/test_plugins/test_geenforce_publish_gate.py new file mode 100644 index 0000000..a1b77e5 --- /dev/null +++ b/tests/test_plugins/test_geenforce_publish_gate.py @@ -0,0 +1,153 @@ +"""Publishing must not outrun the enforcer lib deployed on the fleet. + +THE FAILURE THIS PREVENTS: the engine treats a minor-newer manifest as backward +compatible and carries on. That is true for additions that WIDEN behaviour (an +old lib skips a Type it does not know) and false for one that NARROWS it. The +_CmmVersion filter shipped in lib 2.6 as a minor bump, so a PC on lib 2.5 does +not recognise the field, reads every gated entry as unfiltered, and installs +EVERY PC-DMIS version it cannot detect - on every CMM, within one cycle. + +The share runbook says "push the lib first". A runbook is not a control. +""" + +from datetime import datetime, timezone + +import pytest + +from plugins.geenforce import service +from plugins.geenforce.models import ManifestScope, ManifestEnforcementReport +from shopdb.api import db + + +def _scope(manifestversion='2.6', scopename='gea-shopfloor-cmm'): + scope = ManifestScope(scopename=scopename, phase='runtime', + manifestversion=manifestversion) + db.session.add(scope) + db.session.flush() + return scope + + +def _report(hostname, enforcerversion, scopename='gea-shopfloor-cmm'): + report = ManifestEnforcementReport( + hostname=hostname, scopename=scopename, phase='runtime', + enforcerversion=enforcerversion, receivedat=datetime.now(timezone.utc).replace(tzinfo=None), + iscurrent=True) + db.session.add(report) + db.session.flush() + return report + + +# -- version parsing --------------------------------------------------------- + +@pytest.mark.parametrize('text,expected', [ + ('2.6', (2, 6)), + ('2.10', (2, 10)), # 10 > 6, not string-ordered + ('3', (3, 0)), + (' 2.6 ', (2, 6)), + (None, None), + ('', None), + ('unknown', None), + ('2.x', None), +]) +def test_parse_libversion(text, expected): + assert service.parse_libversion(text) == expected + + +def test_minor_versions_compare_numerically_not_as_strings(): + """'2.10' vs '2.9' is the classic string-compare trap: as text '2.10' sorts + BELOW '2.9' and a current lib would look out of date.""" + assert service.parse_libversion('2.10') > service.parse_libversion('2.9') + + +# -- the gate ---------------------------------------------------------------- + +def test_publish_refused_when_a_cmm_runs_an_older_lib(db): + """The real scenario: cmm manifest 2.6, one bay still on 2.5.""" + _scope('2.6') + _report('CMMBAY01', '2.6') + _report('CMMBAY02', '2.5') + + with pytest.raises(service.LibVersionTooOldError) as caught: + service.publish_scope('gea-shopfloor-cmm', 'runtime') + + error = caught.value + assert error.hosts == ['CMMBAY02'] + assert error.required == '2.6' + assert 'CMMBAY02' in str(error) + # The message has to say what to do, not just that it said no. + assert 'lib' in str(error).lower() + + +def test_publish_allowed_when_the_whole_fleet_is_current(db): + _scope('2.6') + _report('CMMBAY01', '2.6') + _report('CMMBAY02', '2.7') + assert service.publish_scope('gea-shopfloor-cmm', 'runtime') == 1 + + +def test_a_missing_enforcerversion_counts_as_behind(db): + """The field arrived with the summary-emitting engine, so its absence means + an old lib. Treating unknown as safe is how this fails open.""" + _scope('2.6') + _report('CMMBAY01', None) + with pytest.raises(service.LibVersionTooOldError) as caught: + service.publish_scope('gea-shopfloor-cmm', 'runtime') + assert caught.value.hosts == ['CMMBAY01'] + + +def test_an_unparseable_enforcerversion_counts_as_behind(db): + _scope('2.6') + _report('CMMBAY01', 'garbage') + with pytest.raises(service.LibVersionTooOldError): + service.publish_scope('gea-shopfloor-cmm', 'runtime') + + +def test_force_publishes_anyway(db): + """The gate is a guard rail, not a wall - someone who knows why can pass.""" + _scope('2.6') + _report('CMMBAY02', '2.5') + assert service.publish_scope( + 'gea-shopfloor-cmm', 'runtime', force=True) == 1 + + +def test_a_scope_nobody_reports_for_still_publishes(db): + """No reports = no floor to compare. A fresh site must not be unable to + publish anything at all.""" + _scope('2.6') + assert service.publish_scope('gea-shopfloor-cmm', 'runtime') == 1 + + +def test_other_scopes_reports_do_not_block_this_one(db): + """A PC on an old lib reporting for 'common' says nothing about whether the + cmm scope may publish - only PCs that receive THIS scope matter.""" + _scope('2.6') + _report('OLDPC', '2.0', scopename='common') + assert service.publish_scope('gea-shopfloor-cmm', 'runtime') == 1 + + +def test_superseded_reports_are_ignored(db): + """Only the current report per host counts; a stale row from before a lib + upgrade must not block forever.""" + _scope('2.6') + old = _report('CMMBAY01', '2.5') + old.iscurrent = False + _report('CMMBAY01', '2.6') + assert service.publish_scope('gea-shopfloor-cmm', 'runtime') == 1 + + +def test_an_unversioned_manifest_does_not_block(db): + """A scope with no parseable manifestversion has nothing to require.""" + _scope('legacy') + _report('CMMBAY01', '2.5') + assert service.publish_scope('gea-shopfloor-cmm', 'runtime') == 1 + + +def test_hosts_below_libversion_reports_the_floor(db): + _scope('2.6') + _report('A', '2.5') + _report('B', '2.3') + _report('C', '2.6') + hosts, floor = service.hosts_below_libversion( + 'gea-shopfloor-cmm', 'runtime', '2.6') + assert hosts == ['A', 'B'] + assert floor == '2.3' diff --git a/tests/test_plugins/test_geenforce_reporting.py b/tests/test_plugins/test_geenforce_reporting.py index 4ed2d99..0e322b5 100644 --- a/tests/test_plugins/test_geenforce_reporting.py +++ b/tests/test_plugins/test_geenforce_reporting.py @@ -79,9 +79,13 @@ def test_received_latest_flips_when_new_version_published(client, db, app, auth_headers): _seed_and_publish(app) secret = _token(client, auth_headers, ['geenforce.report']) + # enforcerversion matters here: the publish gate refuses to ship a 2.6 + # manifest to a PC that cannot prove it runs at least lib 2.6, and a report + # with no version is treated as an old lib. client.post('/api/geenforce/report', json={'hostname': 'WJCMM02', 'scopename': 'gea-shopfloor-cmm', - 'appliedversion': 1, 'counts': {}}, + 'appliedversion': 1, 'counts': {}, + 'enforcerversion': '2.6'}, headers={'X-API-Key': secret}) # Publish a newer version; the PC is now behind. with app.app_context():