sonic: enforce full config_db.json ownership

osism/tasks/conductor/sonic/config_generator.py

@@ -83,6 +83,38 @@
     "VERSIONS",
 )
 
+# Tables inherited from the device's image-provided base config_db.json:
+# their existing content is preserved across regen because the generator does
+# not emit it itself (see the ownership model on generate_sonic_config).
+INHERITED_TABLE_KEYS = ("DEVICE_METADATA", "VERSIONS")
+
+# Owned tables that are also scaffolded: every scaffold key except the
+# inherited ones. The orchestrator setdefault-creates these up front, so
+# downstream helpers can index into them unconditionally.
+SCAFFOLDED_OWNED_TABLE_KEYS = tuple(
+    key for key in TOP_LEVEL_SCAFFOLD_KEYS if key not in INHERITED_TABLE_KEYS
+)
+
+# Owned tables that are not scaffolded: helpers create and assign them on
+# demand (only when NetBox carries the corresponding data), so when that data
+# is absent the table is simply left out of the generated config.
+ON_DEMAND_OWNED_TABLE_KEYS = (
+    "ROUTE_REDISTRIBUTE",
+    "SNMP_SERVER",
+    "SNMP_AGENT_ADDRESS_CONFIG",
+    "SNMP_SERVER_GROUP_MEMBER",
+    "SNMP_SERVER_USER",
+    "SNMP_SERVER_PARAMS",
+    "SNMP_SERVER_TARGET",
+    "SYSLOG_SERVER",
+)
+
+# Tables fully owned by the generator and rebuilt from scratch on every regen.
+# Their base content is dropped up front so entries removed from NetBox (e.g.
+# a deleted VLAN_MEMBER, VRF, VXLAN_TUNNEL_MAP, or SNMP user) cannot survive as
+# stale config.
+OWNED_TABLE_KEYS = SCAFFOLDED_OWNED_TABLE_KEYS + ON_DEMAND_OWNED_TABLE_KEYS
+
 
 def natural_sort_key(port_name):
     """Extract numeric part from port name for natural sorting."""
@@ -101,6 +133,22 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
 
     Returns:
         dict: Minimal SONiC configuration dictionary
+
+    Config ownership model:
+        This generator owns config_db.json in full; operators must not make
+        manual adjustments to it. On every regen the tables listed in
+        OWNED_TABLE_KEYS are rebuilt from scratch from NetBox data and
+        hardcoded SONiC policy: their base content is dropped up front, so
+        neither pre-existing values nor entries removed from NetBox survive.
+        Operator customizations must be modeled in NetBox or expressed as
+        generator policy, not applied directly to config_db.json.
+
+        The generator builds on the device's image-provided base
+        config_db.json. A few fields it does not itself emit are inherited
+        from that base rather than regenerated — currently the
+        DEVICE_METADATA localhost device attributes (e.g. the device type)
+        and the DATABASE schema VERSION. These come from the image, not from
+        operator hand-edits.
     """
     # Get port configuration for the HWSKU
     port_config = get_port_config(hwsku)
@@ -191,6 +239,13 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
         # Ensure we start fresh even on error
         config = {}
 
+    # Drop any owned-table content carried over from the on-disk base config.
+    # These tables are fully regenerated below from NetBox data and SONiC
+    # policy, so entries removed from NetBox must not survive as stale config.
+    # The inherited tables (DEVICE_METADATA, VERSIONS) keep their base content.
+    for _owned_key in OWNED_TABLE_KEYS:
+        config.pop(_owned_key, None)
+
     # Ensure the top-level scaffold keys the orchestrator and downstream
     # helpers index into directly are always present, even when the on-disk
     # base config is missing or only partially populated.
@@ -218,9 +273,11 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
         primary_ip = str(device.primary_ip6.address).split("/")[0]
 
     if primary_ip:
-        if "default" not in config["BGP_GLOBALS"]:
-            config["BGP_GLOBALS"]["default"] = {}
-        config["BGP_GLOBALS"]["default"]["router_id"] = primary_ip
+        # BGP_GLOBALS is a generated section fully owned by this generator, so
+        # replace the default VRF entry wholesale rather than merging into a
+        # pre-existing one — pre-existing fields from config_db.json must not
+        # survive regen (see the ownership model in the docstring).
+        config["BGP_GLOBALS"]["default"] = {"router_id": primary_ip}
 
         # Calculate and add local_asn from router_id (only for IPv4)
         if device.primary_ip4:
@@ -300,8 +357,6 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
         config["MGMT_INTERFACE"]["eth0"] = {"admin_status": "up"}
         config["MGMT_INTERFACE"][f"eth0|{oob_ip}/{prefix_len}"] = {}
         metalbox_ip = _get_metalbox_ip_for_device(device)
-        # Write into the existing STATIC_ROUTE dict so any pre-existing
-        # routes loaded from /etc/sonic/config_db.json survive.
         config["STATIC_ROUTE"]["mgmt|0.0.0.0/0"] = {"nexthop": metalbox_ip}
     else:
         oob_ip = None

tests/unit/tasks/conductor/sonic/test_config_generator_orchestrator.py

@@ -352,15 +352,16 @@ def test_generate_sonic_config_populates_mgmt_interface_and_static_route(
     assert snmp_oob == "10.42.0.5"
 
 
-def test_generate_sonic_config_oob_path_preserves_existing_static_routes(
+def test_generate_sonic_config_static_route_dropped_on_regen(
     mocker, patch_orchestrator_helpers, make_orchestrator_device
 ):
-    """Pre-existing ``STATIC_ROUTE`` entries must survive the OOB path.
+    """Pre-existing ``STATIC_ROUTE`` entries must be dropped on regen.
 
-    The OOB branch writes the management default route into ``STATIC_ROUTE``;
-    any routes loaded from ``/etc/sonic/config_db.json`` (e.g. an admin's
-    custom blackhole or VRF route) must not be silently dropped when the
-    branch fires.
+    Per the ownership model, STATIC_ROUTE is a generated section fully owned
+    by the generator: it is reset on each regen, so routes loaded from
+    ``/etc/sonic/config_db.json`` (e.g. an operator's custom blackhole or VRF
+    route) do not survive. The OOB branch then writes the management default
+    route as the only entry.
     """
     base = make_base_config()
     base["STATIC_ROUTE"] = {
@@ -374,9 +375,7 @@ def test_generate_sonic_config_oob_path_preserves_existing_static_routes(
 
     config = generate_sonic_config(device, "HWSKU")
 
-    assert config["STATIC_ROUTE"]["mgmt|10.0.0.0/8"] == {"nexthop": "192.0.2.1"}
-    assert config["STATIC_ROUTE"]["default|198.51.100.0/24"] == {"blackhole": "true"}
-    assert config["STATIC_ROUTE"]["mgmt|0.0.0.0/0"] == {"nexthop": "10.42.0.1"}
+    assert config["STATIC_ROUTE"] == {"mgmt|0.0.0.0/0": {"nexthop": "10.42.0.1"}}
 
 
 def test_generate_sonic_config_no_oob_ip_leaves_mgmt_empty_and_passes_none(
@@ -465,6 +464,79 @@ def test_generate_sonic_config_version_existing_in_base_preserved(
     assert config["VERSIONS"]["DATABASE"]["VERSION"] == "version_4_5_0"
 
 
+# ---------------------------------------------------------------------------
+# generate_sonic_config — ownership model: BGP_GLOBALS["default"]
+# ---------------------------------------------------------------------------
+
+
+def test_generate_sonic_config_bgp_globals_default_extra_fields_dropped_on_regen(
+    mocker, patch_orchestrator_helpers, make_orchestrator_device
+):
+    """Pre-existing BGP_GLOBALS['default'] fields must be dropped on regen.
+
+    Per the ownership model, BGP_GLOBALS is a generated section: entries
+    are unconditionally overwritten from NetBox data and hardcoded policy,
+    so pre-existing fields from /etc/sonic/config_db.json must not survive.
+
+    The orchestrator replaces BGP_GLOBALS['default'] wholesale rather than
+    merging into a pre-existing entry, so the default VRF follows the same
+    rule as every other generated section.
+    """
+    base = make_base_config()
+    base["BGP_GLOBALS"]["default"] = {
+        "router_id": "192.0.2.1",
+        "local_asn": "4200000001",
+        "custom_timer": "operator-value",  # not produced by the generator
+    }
+    patch_base_config(mocker, base_config=base)
+    device = make_orchestrator_device(primary_ip4=_ip("10.0.0.1/32"))
+
+    config = generate_sonic_config(device, "HWSKU")
+
+    assert "custom_timer" not in config["BGP_GLOBALS"]["default"]
+
+
+def test_generate_sonic_config_stale_owned_entries_dropped_on_regen(
+    mocker, patch_orchestrator_helpers, make_orchestrator_device
+):
+    """Owned-table entries removed from NetBox must not survive regen.
+
+    The section helpers are mocked here, so nothing repopulates the owned
+    tables: any entry present only because it was carried over from the base
+    config_db.json must be gone after regen. The inherited tables
+    (DEVICE_METADATA, VERSIONS) keep their base content.
+    """
+    base = make_base_config()
+    # Stale entries an operator/earlier run left behind, now absent from NetBox.
+    base["BGP_GLOBALS"]["old-vrf"] = {"router_id": "1.1.1.1"}
+    base["VLAN_MEMBER"]["Vlan999|Ethernet0"] = {"tagging_mode": "tagged"}
+    base["VXLAN_TUNNEL_MAP"]["vtepServ|map_999"] = {"vlan": "Vlan999", "vni": "999"}
+    base["SNMP_SERVER_USER"] = {"olduser": {"shaKey": "x"}}
+    base["SYSLOG_SERVER"] = {"10.9.9.9": {"severity": "info"}}
+    # Inherited tables: must be preserved across regen.
+    base["DEVICE_METADATA"]["localhost"] = {"type": "LeafRouter"}
+    base["VERSIONS"] = {"DATABASE": {"VERSION": "version_4_5_0"}}
+    patch_base_config(mocker, base_config=base)
+    device = make_orchestrator_device(primary_ip4=_ip("10.0.0.1/32"))
+
+    config = generate_sonic_config(device, "HWSKU", config_version=None)
+
+    # Scaffolded owned tables are emptied; the orchestrator rewrites only the
+    # default VRF in BGP_GLOBALS.
+    assert config["BGP_GLOBALS"] == {
+        "default": {"router_id": "10.0.0.1", "local_asn": "4200000001"}
+    }
+    assert config["VLAN_MEMBER"] == {}
+    assert config["VXLAN_TUNNEL_MAP"] == {}
+    # On-demand owned tables are dropped entirely (no mocked helper recreates
+    # them).
+    assert "SNMP_SERVER_USER" not in config
+    assert "SYSLOG_SERVER" not in config
+    # Inherited tables survive untouched.
+    assert config["DEVICE_METADATA"]["localhost"]["type"] == "LeafRouter"
+    assert config["VERSIONS"]["DATABASE"]["VERSION"] == "version_4_5_0"
+
+
 # ---------------------------------------------------------------------------
 # generate_sonic_config — netbox_interfaces collection
 # ---------------------------------------------------------------------------