geenforce: gate publishing on the library version, not on the manifest's own
The publish gate exists because a minor version bump that NARROWS behaviour is not backward compatible: _CmmVersion arrived in lib 2.6, and 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, in one cycle. It was comparing the fleet's reported library versions against manifestversion. That is the manifest's own 'Version' field. For a share-imported manifest the two numbering schemes happen to coincide; for a scope authored in code they do not, and seed_display_scope writes '2.0' - which every kiosk exceeds. So the gate passed on the scope that most needed it. A scope now declares minlibversion. Unset, the requirement is DERIVED from what the manifest actually uses, so a scope written before this column existed is still judged on its contents rather than on a number that says nothing about the library. Only features that narrow behaviour belong in that table; an addition an old lib ignores harmlessly needs no floor. manifestversion remains the last fallback, which preserves what share-imported manifests already relied on.
This commit is contained in:
@@ -352,6 +352,8 @@ def list_scopes():
|
||||
'scopename': scope.scopename,
|
||||
'phase': scope.phase,
|
||||
'manifestversion': scope.manifestversion,
|
||||
'minlibversion': scope.minlibversion,
|
||||
'minlibversion': scope.minlibversion,
|
||||
'computertypeid': scope.computertypeid,
|
||||
'measuringtooltypeid': scope.measuringtooltypeid,
|
||||
'iscommon': scope.iscommon,
|
||||
@@ -462,6 +464,7 @@ def create_scope():
|
||||
scope = ManifestScope(
|
||||
scopename=scopename, phase=phase,
|
||||
manifestversion=str(payload.get('manifestversion', '1.0')),
|
||||
minlibversion=(payload.get('minlibversion') or None),
|
||||
description=payload.get('description'),
|
||||
computertypeid=payload.get('computertypeid'),
|
||||
measuringtooltypeid=payload.get('measuringtooltypeid'),
|
||||
@@ -496,6 +499,10 @@ def update_scope(scopeid):
|
||||
setattr(scope, field, payload[field])
|
||||
if 'manifestversion' in payload:
|
||||
scope.manifestversion = str(payload['manifestversion'])
|
||||
if 'minlibversion' in payload:
|
||||
# Blank clears it, which returns the scope to a derived floor rather
|
||||
# than pinning it at the empty string.
|
||||
scope.minlibversion = (payload['minlibversion'] or None)
|
||||
if 'iscommon' in payload:
|
||||
scope.iscommon = bool(payload['iscommon'])
|
||||
db.session.commit()
|
||||
@@ -764,9 +771,9 @@ def publish_preflight(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)
|
||||
scope.scopename, scope.phase, service.required_libversion(scope))
|
||||
return success_response({
|
||||
'required': scope.manifestversion,
|
||||
'required': service.required_libversion(scope),
|
||||
'floor': floor,
|
||||
'hostsbehind': hosts,
|
||||
'canpublish': not hosts,
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
"""A scope declares the client library it needs, separately from its own version.
|
||||
|
||||
The publish gate exists to refuse a manifest the fleet's GE-Enforce library
|
||||
cannot read correctly. It was comparing reported library versions against
|
||||
`manifestversion` - the manifest's own 'Version' field, which for a share
|
||||
imported manifest happens to share the library's numbering but for a
|
||||
code-authored scope is whatever the author chose (seed_display_scope writes
|
||||
'2.0'). Every kiosk runs something newer than that, so the gate passed on the
|
||||
one scope most in need of it.
|
||||
|
||||
`minlibversion` says the thing the gate actually wants to know. NULL means
|
||||
derive it from what the manifest uses, so existing scopes need no edit.
|
||||
|
||||
Revision ID: geenforce0004minlib
|
||||
Revises: geenforce0003subtype
|
||||
"""
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision = 'geenforce0004minlib'
|
||||
down_revision = 'geenforce0003subtype'
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
TABLE = 'manifestscopes'
|
||||
|
||||
|
||||
def _columns(bind):
|
||||
insp = sa.inspect(bind)
|
||||
if TABLE not in insp.get_table_names():
|
||||
return None
|
||||
return {c['name'] for c in insp.get_columns(TABLE)}
|
||||
|
||||
|
||||
def upgrade():
|
||||
# Guarded like geenforce0003subtype and network0003prefix: on a fresh
|
||||
# database the table is built from the models, which already declare this.
|
||||
columns = _columns(op.get_bind())
|
||||
if columns is None or 'minlibversion' in columns:
|
||||
return
|
||||
op.add_column(TABLE, sa.Column('minlibversion', sa.String(length=16),
|
||||
nullable=True))
|
||||
|
||||
|
||||
def downgrade():
|
||||
columns = _columns(op.get_bind())
|
||||
if columns is None or 'minlibversion' not in columns:
|
||||
return
|
||||
op.drop_column(TABLE, 'minlibversion')
|
||||
@@ -49,6 +49,15 @@ class ManifestScope(BaseModel):
|
||||
computertypeid = db.Column(db.Integer, nullable=True)
|
||||
measuringtooltypeid = db.Column(db.Integer, nullable=True)
|
||||
manifestversion = db.Column(db.String(16), nullable=False, default='1.0')
|
||||
# Lowest GE-Enforce client library that may enforce this scope, e.g. '2.6'.
|
||||
#
|
||||
# SEPARATE FROM manifestversion, which is the manifest's own 'Version' field
|
||||
# and is whatever its author wrote - seed_display_scope picks '2.0'. The
|
||||
# publish gate used manifestversion as the library floor, so for every
|
||||
# code-authored scope it compared the fleet against a number that says
|
||||
# nothing about library features, and passed. NULL means "derive it", see
|
||||
# service.required_libversion.
|
||||
minlibversion = db.Column(db.String(16), nullable=True)
|
||||
description = db.Column(db.String(255), nullable=True)
|
||||
# The manifest-level '_comment' (documentation), preserved verbatim so
|
||||
# export-to-share round-trips it. Excluded from behavioral parity.
|
||||
|
||||
@@ -116,6 +116,45 @@ def hosts_below_libversion(scopename, phase, required):
|
||||
return sorted(set(behind)), floor
|
||||
|
||||
|
||||
# Manifest features and the client library that first understood them. A
|
||||
# library older than this does not merely skip the feature, it MISREADS the
|
||||
# entry: 2.6 added _CmmVersion, and an older lib treats a version-gated entry as
|
||||
# ungated and installs every PC-DMIS build it cannot detect.
|
||||
#
|
||||
# Only features that NARROW behaviour belong here. An addition an old lib
|
||||
# ignores harmlessly does not need a floor.
|
||||
LIBVERSION_FEATURES = (
|
||||
('cmmversion', '2.6'),
|
||||
)
|
||||
|
||||
|
||||
def required_libversion(scope):
|
||||
"""Lowest client library that may enforce this scope.
|
||||
|
||||
An explicit minlibversion wins. Otherwise it is derived from the features
|
||||
the draft actually uses, so a scope written before this column existed is
|
||||
still gated on what it contains rather than on manifestversion - which is
|
||||
the manifest's own 'Version' field and says nothing about the library.
|
||||
|
||||
Falls back to manifestversion when nothing else applies, preserving the
|
||||
behaviour share-imported manifests already relied on, where the two
|
||||
numbering schemes do coincide.
|
||||
"""
|
||||
declared = (scope.minlibversion or '').strip()
|
||||
if declared:
|
||||
return declared
|
||||
|
||||
floor = None
|
||||
for attribute, version in LIBVERSION_FEATURES:
|
||||
if any(getattr(entry, attribute, None) for entry in scope.entries):
|
||||
candidate = parse_libversion(version)
|
||||
if candidate and (floor is None or candidate > floor[0]):
|
||||
floor = (candidate, version)
|
||||
if floor:
|
||||
return floor[1]
|
||||
return scope.manifestversion
|
||||
|
||||
|
||||
def _installed_app_model():
|
||||
"""Lazily import the computers plugin's ComputerInstalledApp.
|
||||
|
||||
@@ -184,11 +223,11 @@ def publish_scope(scopename, phase, notes=None, publishedby=None, force=False):
|
||||
raise ValueError(f'No scope {scopename}/{phase}')
|
||||
|
||||
if not force:
|
||||
behind, floor = hosts_below_libversion(
|
||||
scopename, phase, scope.manifestversion)
|
||||
required = required_libversion(scope)
|
||||
behind, floor = hosts_below_libversion(scopename, phase, required)
|
||||
if behind:
|
||||
raise LibVersionTooOldError(
|
||||
scopename, scope.manifestversion, floor or 'unknown', behind)
|
||||
scopename, required, floor or 'unknown', behind)
|
||||
|
||||
text = scope_to_json(scope)
|
||||
maxversion = db.session.query(
|
||||
|
||||
@@ -263,3 +263,42 @@ def test_reported_subtype_keeps_an_unknown_value_verbatim(db):
|
||||
stored = ManifestEnforcementReport.query.filter_by(
|
||||
hostname='WJKIOSKODD', iscurrent=True).one()
|
||||
assert stored.subtype == 'Lobbby'
|
||||
|
||||
|
||||
# -- what the gate compares against ------------------------------------------
|
||||
#
|
||||
# The gate used manifestversion as the library floor. That is the manifest's own
|
||||
# 'Version' field: for a share-imported manifest it happens to share the
|
||||
# library's numbering, but a code-authored scope sets it freely -
|
||||
# seed_display_scope writes '2.0' - so the fleet was compared against a number
|
||||
# that says nothing about library features, and the gate passed on the very
|
||||
# scope that most needed it.
|
||||
|
||||
def test_a_declared_minlibversion_is_what_the_fleet_is_judged_against(db):
|
||||
scope = _scope(manifestversion='2.0')
|
||||
scope.minlibversion = '2.6'
|
||||
db.session.flush()
|
||||
assert service.required_libversion(scope) == '2.6'
|
||||
|
||||
_report('KIOSK01', '2.4')
|
||||
with pytest.raises(service.LibVersionTooOldError):
|
||||
service.publish_scope('gea-shopfloor-cmm', 'runtime')
|
||||
|
||||
|
||||
def test_the_floor_is_derived_from_the_features_the_manifest_uses(db):
|
||||
"""A scope written before minlibversion existed is still gated on what it
|
||||
contains: a version-gated entry needs lib 2.6, whatever its own Version
|
||||
field claims."""
|
||||
from plugins.geenforce.models import ManifestEntry
|
||||
|
||||
scope = _scope(manifestversion='2.0')
|
||||
scope.entries.append(ManifestEntry(
|
||||
name='PC-DMIS 2023.2', entrytype='MSI', sortorder=0,
|
||||
cmmversion='2023'))
|
||||
db.session.flush()
|
||||
assert service.required_libversion(scope) == '2.6'
|
||||
|
||||
|
||||
def test_a_scope_using_nothing_special_falls_back_to_its_own_version(db):
|
||||
scope = _scope(manifestversion='2.4')
|
||||
assert service.required_libversion(scope) == '2.4'
|
||||
|
||||
Reference in New Issue
Block a user