Let the server decide which NIC is a PC's address, and keep the rest

A CMM reported 100.0.0.1 as its address and never reported 10.48.255.210 at
all. Both halves of that were the same mistake.

THE BAY WAS CHOOSING. The collector schema took a single `ipaddress`, so the
script picked one NIC and dropped the others - preferring an address inside a
configured range, else whichever interface carried a default route. A bay
cannot make that judgement: it does not know the site's addressing. On this
CMM the Marvell AQtion 10Gbit link to its controller is a real hardware NIC,
up, and carries a default route of its own, so it satisfied every test the
script had. It won, and the corporate address was discarded before the request
was even built - which is why it is not in ShopDB even as a second row.

So the bay now reports EVERY connected hardware NIC with a routable IPv4, and
takes no view. The server picks the primary from collector_corporate_ranges, a
new per-site setting, and writes a Communication row for each of the others.
Naming the ranges is what turns the answer from a guess into a decision; with
none set it still falls back to the default-route flag and then to the first
reported, both guesses, both better than before only because nothing is thrown
away now.

Rows are keyed on the MAC where the bay sends one, because an address moves
between adapters and a MAC does not. A NIC that is down at check-in is left
alone rather than deleted - being down is not evidence the record is wrong,
and hand-entered gateway, DNS and serial settings live on those same rows.
Exactly one row ends up flagged primary, and only a row this report touched
can claim it, so a check-in cannot silently demote an address set by hand.

-AllowedRanges and the CollectorRanges registry value are now accepted and
ignored, logging a line that says where the setting moved. The parameter stays
because a scheduled task still passing it would otherwise fail on an unknown
argument.

A bay on an older collector sends one address and no interfaces; that keeps
working and is pinned by a test.

Scope: product

plugins/computers/plugin.py

@@ -1,5 +1,6 @@
 """Computers plugin main class."""
 
+import ipaddress
 import json
 import logging
 from pathlib import Path
@@ -59,6 +60,115 @@ PARTMARKER_LINK_ORIGIN = 'collector:partmarker'
 MACHINE_CLAIM_QUIET_HOURS = 24
 
 
+def _corporate_ranges():
+    """CIDRs this site considers its routable network, from settings.
+
+    Empty means the site has not said, and the fallback below applies. Kept
+    server-side rather than pushed to the bay: a bay cannot know a site's
+    addressing, and baking a range into the collector script would be an
+    ADR-015 violation the moment a second site ran it.
+    """
+    from shopdb.api import Setting
+    raw = ''
+    try:
+        raw = Setting.get('collector_corporate_ranges') or ''
+    except Exception:
+        return []
+    ranges = []
+    for cidr in str(raw).split(','):
+        cidr = cidr.strip()
+        if not cidr:
+            continue
+        try:
+            ranges.append(ipaddress.ip_network(cidr, strict=False))
+        except ValueError:
+            continue
+    return ranges
+
+
+def _pick_primary(interfaces, ranges):
+    """Which reported NIC is the PC's address.
+
+    In order: inside a configured corporate range; else the one the bay says
+    carries the default route; else the first. The last is a guess and is the
+    reason configuring the ranges is worth doing - it is what a site sets so
+    the answer stops being a guess.
+    """
+    def addr(entry):
+        try:
+            return ipaddress.ip_address((entry.get('ipaddress') or '').strip())
+        except ValueError:
+            return None
+
+    if ranges:
+        for entry in interfaces:
+            parsed = addr(entry)
+            if parsed and any(parsed in net for net in ranges):
+                return entry
+    for entry in interfaces:
+        if entry.get('isdefaultroute'):
+            return entry
+    return interfaces[0] if interfaces else None
+
+
+def _record_interfaces(comp, interfaces):
+    """Write one Communication row per reported NIC, flagging exactly one.
+
+    Keyed on the MAC where the bay sent one, because an address moves between
+    adapters and a MAC does not. Falls back to the address for a bay that
+    reports no MAC, which is what the single-ipaddress payload looks like.
+
+    Rows this PC has that the bay did NOT report are left alone: a NIC that is
+    down at check-in time is not evidence the record is wrong, and hand-entered
+    detail (gateway, DNS, serial settings) lives on these same rows.
+    """
+    # Imported here for the same reason the caller does: these come through the
+    # shopdb.api contract surface, not from core internals.
+    from shopdb.api import Communication, CommunicationType, db
+
+    warnings = []
+    ip_comtype = CommunicationType.query.filter_by(comtype='IP').first()
+    if not ip_comtype:
+        return ['no IP communication type seeded; network not recorded']
+
+    usable = [entry for entry in interfaces if (entry.get('ipaddress') or '').strip()]
+    if not usable:
+        return warnings
+
+    primary = _pick_primary(usable, _corporate_ranges())
+    existing = Communication.query.filter_by(assetid=comp.assetid).all()
+    bymac = {(c.macaddress or '').strip().lower(): c for c in existing if c.macaddress}
+    byip = {(c.ipaddress or '').strip(): c for c in existing if c.ipaddress}
+
+    touched = []
+    for entry in usable:
+        addr = (entry.get('ipaddress') or '').strip()
+        mac = (entry.get('macaddress') or '').strip()
+        row = bymac.get(mac.lower()) if mac else None
+        if row is None:
+            row = byip.get(addr)
+        if row is None:
+            row = Communication(assetid=comp.assetid, comtypeid=ip_comtype.comtypeid)
+            db.session.add(row)
+        row.ipaddress = addr
+        if mac:
+            row.macaddress = mac
+        touched.append((row, entry is primary))
+
+    # Exactly one primary, and only among rows this report touched. An untouched
+    # row keeps its flag only if nothing here claimed it, which is what stops a
+    # check-in from silently demoting an address somebody set by hand.
+    for row, isprimary in touched:
+        row.isprimary = isprimary
+    if any(isprimary for _, isprimary in touched):
+        claimed = {id(row) for row, _ in touched}
+        for row in existing:
+            if id(row) not in claimed:
+                row.isprimary = False
+
+    return warnings
+
+
 class ComputersPlugin(BasePlugin):
     """
     Computers plugin - manages PC, server, and workstation assets.
@@ -362,7 +472,27 @@ class ComputersPlugin(BasePlugin):
                 'loggedinuser': {'type': 'string'},
                 'lastboottime': {'type': 'string', 'format': 'date-time'},
                 'lastcheckin': {'type': 'string', 'format': 'date-time'},
+                # Kept for bays on an older collector. A payload carrying
+                # both wins on `interfaces`.
                 'ipaddress': {'type': 'string'},
+                # EVERY routable IPv4 the bay can see, and no opinion about
+                # which one matters. Choosing was the bay's job and it was the
+                # wrong place for it: the bay cannot know a site's addressing,
+                # so a CMM with a 10Gbit link to its controller reported the
+                # machine-side address as the PC's, and the corporate address
+                # was never sent at all. The server picks the primary now,
+                # using collector_corporate_ranges, and records the rest.
+                'interfaces': {
+                    'type': 'array',
+                    'items': {
+                        'ipaddress': 'string',
+                        'macaddress': 'string',
+                        'adaptername': 'string',
+                        'isdefaultroute': 'boolean',
+                    },
+                    'description': ('All connected hardware NICs with a '
+                                    'routable IPv4. Order carries no meaning.'),
+                },
                 'vendorname': {'type': 'string'},
                 'modelnumber': {'type': 'string'},
                 'osname': {'type': 'string'},
@@ -446,6 +576,26 @@ class ComputersPlugin(BasePlugin):
         site that installed an earlier one.
         """
         return [
+            {
+                # Which addresses are THIS site's routable network. The
+                # collector reports every NIC and takes no view; this decides
+                # which one is the PC's address.
+                #
+                # Empty falls back to the NIC the bay says carries the default
+                # route, then to the first reported - both guesses. A CMM with
+                # a 10Gbit link to its controller is the case that shows why:
+                # that link can carry a default route too, and the machine-side
+                # address then wins over the corporate one.
+                #
+                # Comma-separated CIDRs, e.g. '10.48.0.0/16,10.20.0.0/23'.
+                'key': 'collector_corporate_ranges',
+                'value': '',
+                'valuetype': 'string',
+                'category': 'computers',
+                'description': ('Comma-separated CIDRs naming this site routable '
+                                'network. The collector reports every NIC; this '
+                                'picks which is the primary address.'),
+            },
             {
                 'key': 'computers_quietreporthours',
                 'value': '24',
@@ -646,16 +796,18 @@ class ComputersPlugin(BasePlugin):
         if payload.get('pcsubtype'):
             warnings.append('pcsubtype received but not stored (no model field)')
 
-        if payload.get('ipaddress'):
-            ip_comtype = CommunicationType.query.filter_by(comtype='IP').first()
-            primary = Communication.query.filter_by(
-                assetid=comp.assetid, isprimary=True).first()
-            if primary:
-                primary.ipaddress = payload['ipaddress']
-            elif ip_comtype:
-                db.session.add(Communication(
-                    assetid=comp.assetid, comtypeid=ip_comtype.comtypeid,
-                    ipaddress=payload['ipaddress'], isprimary=True))
+        # NETWORK. A bay reports every NIC it can see; THIS decides which one
+        # is the PC's address, because only the server knows the site's
+        # addressing. A CMM whose 10Gbit link to its controller sits on
+        # 100.0.0.1 used to report that as the PC's address and never send the
+        # corporate one at all - the bay had no way to tell them apart.
+        interfaces = payload.get('interfaces') or []
+        if not interfaces and payload.get('ipaddress'):
+            # An older bay sends one address and no opinion beyond it.
+            interfaces = [{'ipaddress': payload['ipaddress']}]
+
+        if interfaces:
+            warnings.extend(_record_interfaces(comp, interfaces))
 
         for app_data in payload.get('installedsoftware', []) or []:
             name = app_data.get('name')