Correct base config provenance and make its path configurable

osism/settings.py

@@ -72,6 +72,16 @@ def read_secret(secret_name):
     DEFAULT_NETBOX_FILTER_CONDUCTOR_SONIC,
 )
 
+# Base config_db.json the SONiC config generator builds every device config on.
+# Shipped in this repo as files/sonic/config_db.json and installed at this path
+# by the Containerfile, so the default is correct inside the conductor image.
+# Overridable so a generator running outside that image -- a test harness, a
+# pip install -- can point at the in-repo copy instead of silently generating
+# without a base config (see the ownership model on generate_sonic_config).
+SONIC_BASE_CONFIG_PATH = os.getenv(
+    "SONIC_BASE_CONFIG_PATH", "/etc/sonic/config_db.json"
+)
+
 # SONiC export configuration
 SONIC_EXPORT_DIR = os.getenv("SONIC_EXPORT_DIR", "/etc/sonic/export")
 SONIC_EXPORT_PREFIX = os.getenv("SONIC_EXPORT_PREFIX", "osism_")

osism/tasks/conductor/sonic/config_generator.py

@@ -11,7 +11,7 @@
 from typing import Optional
 from loguru import logger
 
-from osism import utils
+from osism import settings, utils
 from osism.tasks.conductor.netbox import (
     get_device_interface_ips,
     get_device_loopbacks,
@@ -95,26 +95,24 @@
     "VERSIONS",
 )
 
-# Tables inherited from the device's image-provided base config_db.json: not
-# dropped on regen, so their base content is preserved, while the generator
-# updates only selected fields in place (DEVICE_METADATA localhost attributes,
-# VERSIONS.DATABASE.VERSION) rather than rebuilding them wholesale (see the
-# ownership model on generate_sonic_config).
+# Tables inherited from the base config_db.json: not dropped on regen, so their
+# base content is preserved, while the generator updates only selected fields in
+# place (DEVICE_METADATA localhost attributes, VERSIONS.DATABASE.VERSION) rather
+# than rebuilding them wholesale (see the ownership model on
+# generate_sonic_config).
 INHERITED_TABLE_KEYS = ("DEVICE_METADATA", "VERSIONS")
 
-# Tables read from the device's image-provided base config_db.json but never
-# modified by the generator: their values are consumed (via a defensive
-# config.get(...), so a missing table is tolerated). They are neither dropped
-# nor written, so an image-provided table passes through to the output
-# unchanged -- the same output behaviour as a pass-through table, the
-# difference being only that the generator depends on this one as an input.
-# Distinct from inherited tables, which the generator also writes selected
+# Tables read from the base config_db.json but never modified by the generator:
+# their values are consumed (via a defensive config.get(...), so a missing table
+# is tolerated). They are neither dropped nor written, so such a table passes
+# through to the output unchanged -- the same output behaviour as a pass-through
+# table, the difference being only that the generator depends on this one as an
+# input. Distinct from inherited tables, which the generator also writes selected
 # fields into; the distinction is read-only vs. read-and-update, not the access
-# syntax (inherited tables are read via config.get() too). For example, the
-# gNMI listen port is read from TELEMETRY. Must stay disjoint from
-# OWNED_TABLE_KEYS -- an image-consumed table dropped up front would always
-# read back empty.
-IMAGE_CONSUMED_TABLE_KEYS = ("TELEMETRY",)
+# syntax (inherited tables are read via config.get() too). For example, the gNMI
+# listen port is read from TELEMETRY. Must stay disjoint from OWNED_TABLE_KEYS --
+# a read-only table dropped up front would always read back empty.
+READ_ONLY_TABLE_KEYS = ("TELEMETRY",)
 
 # Owned tables that are also scaffolded: every scaffold key except the
 # inherited ones. The orchestrator setdefault-creates these up front, so
@@ -175,12 +173,15 @@
 # Configuration of the default VRF, emitted by _add_default_vrf_configuration
 # on every regen.
 #
-# These entries used to ship in the image-provided base config_db.json, with
-# the generator adding only the VRF-specific ones. The tables holding them are
-# owned, so their base content is dropped up front -- which silently removed
-# the default-VRF entries on every regen and, with them, the default VRF's
-# EVPN route advertisement. The generator therefore owns these entries too;
-# the values are the ones the base config used to carry.
+# These entries used to ship in the base config_db.json (files/sonic/config_db.json,
+# installed into the conductor image at /etc/sonic/config_db.json), with the
+# generator adding only the VRF-specific ones. The tables holding them are owned,
+# so their base content is dropped up front -- which silently removed the
+# default-VRF entries on every regen and, with them, the default VRF's EVPN route
+# advertisement (#2515). The generator therefore owns these entries too; the
+# values are the ones the base config used to carry. Policy constants are where
+# such defaults belong -- see "Where defaults belong" in the ownership model on
+# generate_sonic_config.
 #
 # The mappings are read-only views: they are module-level state shared by every
 # device, so an in-place edit would rewrite the policy for the whole process.
@@ -271,35 +272,62 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
         dict: Minimal SONiC configuration dictionary
 
     Config ownership model:
-        The generator builds on the device's image-provided base
-        config_db.json and classifies every table it touches into one of four
-        categories. Operator hand-edits to config_db.json are unsupported in
-        all of them: customizations must be modeled in NetBox or expressed as
-        generator policy, never applied directly to the file.
+        The generator builds on a base config_db.json and classifies every
+        table it touches into one of four categories.
+
+        What the base config is: this repo's files/sonic/config_db.json, which
+        the Containerfile installs into the conductor image at
+        /etc/sonic/config_db.json. The generator reads that path from its own
+        container filesystem. It is *not* the switch's SONiC image config and
+        not a per-device file — nothing from a device's own config_db.json ever
+        reaches the generator. Everything device-specific comes from NetBox and
+        the policy in this module; the base config supplies the shared content
+        of the inherited, read-only and pass-through tables below, which
+        nothing else provides. Because the base config is baked into a
+        container image, it is not an operator-editable surface either:
+        customizations must be modeled in NetBox or expressed as generator
+        policy, never applied directly to the file.
 
         - Owned (OWNED_TABLE_KEYS): fully owned by the generator and rebuilt
           from scratch every regen 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.
-        - Inherited (INHERITED_TABLE_KEYS): not dropped on regen, so the
-          image base content is preserved, while the generator updates
-          selected fields in place — currently DEVICE_METADATA localhost
-          attributes (hostname, hwsku, platform, mac) and the
-          VERSIONS.DATABASE.VERSION. Scaffold-created when absent, so they
-          always exist at access time even on a fresh base config.
-        - Image-consumed (IMAGE_CONSUMED_TABLE_KEYS): read from the image
-          base config but never modified by the generator. The defining
-          property is behavioural (read-only), not the access syntax:
-          inherited tables are also read via config.get(), so .get() alone
-          does not distinguish the two. IMAGE_CONSUMED_TABLE_KEYS lists the
-          current members.
+        - Inherited (INHERITED_TABLE_KEYS): not dropped on regen, so the base
+          content is preserved, while the generator updates selected fields in
+          place — currently DEVICE_METADATA localhost attributes (hostname,
+          hwsku, platform, mac) and the VERSIONS.DATABASE.VERSION.
+          Scaffold-created when absent, so they always exist at access time
+          even on a fresh base config.
+        - Read-only (READ_ONLY_TABLE_KEYS): read from the base config but
+          never modified by the generator. The defining property is
+          behavioural, not the access syntax: inherited tables are also read
+          via config.get(), so .get() alone does not distinguish the two.
+          READ_ONLY_TABLE_KEYS lists the current members.
         - Pass-through: every table the generator never references. Left
           untouched and unmanaged; not a supported place for operator
           customizations either.
 
+        Where defaults belong: owned tables are dropped before any helper
+        runs, so a populated *owned* table in files/sonic/config_db.json is
+        dead content — silently discarded on every regen. Defaults the
+        generator needs must live in this module as policy constants (see the
+        DEFAULT_VRF_* mappings), never in the base config. #2515 was exactly
+        this mistake: the default-VRF BGP entries shipped in the base config,
+        the ownership drop removed them, and nothing regenerated them, so the
+        default VRF stopped advertising its routes into EVPN. Only inherited,
+        read-only and pass-through tables carry meaningful base content.
+        The rule is about owned tables specifically: a read-only table such
+        as TELEMETRY is a dependency that legitimately lives only in the base
+        config, because the generator reads it rather than emitting it.
+
+        The base config ships *only* in the container image — files/ is not
+        Python package data — so a generator running outside that image (pip
+        install, test harness) finds nothing and falls back to the hardcoded
+        defaults. Point SONIC_BASE_CONFIG_PATH at the in-repo copy there.
+
         A static guard test (see test_config_generator_ownership.py) parses
         this module and fails the build if it references a table that is not
-        owned, inherited, or image-consumed — so a newly handled table cannot
+        owned, inherited, or read-only — so a newly handled table cannot
         silently fall into the unpoliced pass-through tier and reintroduce
         stale config.
 
@@ -382,9 +410,9 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
     hostname = get_device_hostname(device)
     mac_address = get_device_mac_address(device)
 
-    # Try to load base configuration from /etc/sonic/config_db.json
+    # Try to load the base configuration (see the ownership model above)
     # Always start with a fresh, empty configuration for each device
-    base_config_path = "/etc/sonic/config_db.json"
+    base_config_path = settings.SONIC_BASE_CONFIG_PATH
     config = {}
 
     try:
@@ -397,7 +425,19 @@ def generate_sonic_config(device, hwsku, device_as_mapping=None, config_version=
                     f"Loaded fresh base configuration from {base_config_path} for device {device.name}"
                 )
         else:
-            logger.debug(
+            # Warn rather than debug: the base config supplies content nothing
+            # else does, so the result is incomplete in ways that are easy to
+            # miss. Absent are the DEVICE_METADATA localhost fields the
+            # generator does not itself write (type, default_config_profile,
+            # frr_mgmt_framework_config, intf_naming_mode) and every
+            # pass-through table (FEATURE, CLASSIFIER_TABLE, the POLICY_* set,
+            # ...); the read-only TELEMETRY gNMI port falls back to
+            # DEFAULT_GNMI_PORT. What the generator writes itself is unaffected
+            # -- the localhost attributes it owns (hostname, hwsku, platform,
+            # mac) and the DATABASE VERSION default -- which is precisely why
+            # the gap is easy to overlook downstream. Set
+            # SONIC_BASE_CONFIG_PATH when running outside the conductor image.
+            logger.warning(
                 f"Base config file {base_config_path} not found, starting with empty config for device {device.name}"
             )
     except Exception as e:
@@ -2467,7 +2507,7 @@ def _get_gnmi_port(config):
 
     The telemetry/gNMI container reads its listen port from
     TELEMETRY|gnmi|port and falls back to 8080 when unset, so the ACL rule
-    follows the same lookup against the (image-consumed) TELEMETRY table,
+    follows the same lookup against the (read-only) TELEMETRY table,
     treating a present-but-empty value (null, "") as unset like the
     container does. A malformed value (non-numeric or outside 1-65535)
     also falls back to the default, with a warning, rather than

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

@@ -37,16 +37,18 @@ def patch_base_config(mocker, *, exists=True, base_config=None, raise_on_open=No
     - ``exists=False`` → ``open`` is not patched (the orchestrator never
       reaches the ``with open`` path).
     - ``raise_on_open`` → ``open`` raises this exception (e.g. ``OSError``).
+
+    Returns the patched ``open`` mock so a caller can assert on the path it was
+    called with, or ``None`` when ``exists=False`` and ``open`` is left alone.
     """
 
     mocker.patch.object(config_generator.os.path, "exists", return_value=exists)
     if not exists:
-        return
+        return None
     if raise_on_open is not None:
-        mocker.patch("builtins.open", side_effect=raise_on_open)
-        return
+        return mocker.patch("builtins.open", side_effect=raise_on_open)
     cfg = base_config if base_config is not None else make_base_config()
-    mocker.patch("builtins.open", mock_open(read_data=json.dumps(cfg)))
+    return mocker.patch("builtins.open", mock_open(read_data=json.dumps(cfg)))
 
 
 def make_iface(name, *, mgmt_only=False, type_value=None, iface_id=None):

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

@@ -223,6 +223,54 @@ def test_generate_sonic_config_preserves_existing_localhost_keys(
 # ---------------------------------------------------------------------------
 
 
+def test_generate_sonic_config_loads_base_from_configured_path(
+    mocker, patch_orchestrator_helpers, make_orchestrator_device
+):
+    """The base config is read from settings.SONIC_BASE_CONFIG_PATH.
+
+    The path is overridable so a generator running outside the conductor image
+    (test harness, pip install) can point at the in-repo copy instead of
+    silently generating without a base config.
+    """
+    mocker.patch.object(
+        config_generator.settings,
+        "SONIC_BASE_CONFIG_PATH",
+        "/custom/base/config_db.json",
+    )
+    opener = patch_base_config(
+        mocker, base_config=make_base_config(version="version_4_2_0")
+    )
+    device = make_orchestrator_device()
+
+    config = generate_sonic_config(device, "Test-HWSKU", config_version=None)
+
+    assert config["VERSIONS"]["DATABASE"]["VERSION"] == "version_4_2_0"
+    assert opener.call_args_list[0].args[0] == "/custom/base/config_db.json"
+
+
+def test_generate_sonic_config_warns_when_base_absent(
+    mocker, patch_orchestrator_helpers, make_orchestrator_device
+):
+    """A missing base config is a warning, not a debug line.
+
+    Without it the config is missing the DEVICE_METADATA localhost fields the
+    generator does not write itself (type, default_config_profile, …) and every
+    pass-through table, while TELEMETRY falls back to DEFAULT_GNMI_PORT. What
+    the generator writes itself still lands, so the gap is easy to miss
+    downstream — the E2E harness generated against no base config for months.
+    """
+    patch_base_config(mocker, exists=False)
+    warning = mocker.patch.object(config_generator.logger, "warning")
+    device = make_orchestrator_device()
+
+    generate_sonic_config(device, "Test-HWSKU")
+
+    assert any(
+        "not found, starting with empty config" in str(call)
+        for call in warning.call_args_list
+    )
+
+
 def test_generate_sonic_config_starts_from_scaffold_when_base_absent(
     mocker, patch_orchestrator_helpers, make_orchestrator_device
 ):
@@ -428,7 +476,7 @@ def test_generate_sonic_config_gnmi_port_survives_owned_table_drop_end_to_end(
 ):
     """End-to-end with the real ACL helper: a TELEMETRY|gnmi|port from the
     base config must reach the generated GNMI_ONLY rule. TELEMETRY is
-    image-consumed while ACL_TABLE/ACL_RULE are owned and dropped up front,
+    read-only while ACL_TABLE/ACL_RULE are owned and dropped up front,
     so this pins the whole flow -- the port is read after the drop and
     emitted into the final output -- which the wiring tests above (helper
     mocked) and the helper unit tests (no orchestrator) only cover
@@ -555,7 +603,7 @@ def test_generate_sonic_config_emits_default_vrf_entries_without_vni_vrfs(
     """A device without VNI VRFs still gets the full default-VRF config.
 
     VRF, the BGP_GLOBALS* tables and ROUTE_REDISTRIBUTE are owned, so whatever
-    the image base config_db.json carries for the default VRF is dropped on
+    the base config_db.json carries for the default VRF is dropped on
     regen, and ``_add_vrf_configuration`` only writes entries for the VRFs
     NetBox carries. ``_add_default_vrf_configuration`` therefore emits the
     default-VRF entries — without them the default VRF stops advertising its