fix(sonic): keep AF admin_status true/false

Giltfile.yaml

@@ -23,8 +23,9 @@
 #
 # Note also that the models are community SONiC while the supported HWSKUs run
 # Enterprise SONiC builds. Where the two disagree the table is opted out via
-# PLATFORM_DIVERGENT_TABLES in tools/sonic_yang_to_pydantic.py; re-check that
-# list when this pin moves.
+# PLATFORM_DIVERGENT_TABLES in tools/sonic_yang_to_pydantic.py, or a single leaf
+# is retyped via PLATFORM_DIVERGENT_FIELDS beside it; re-check both lists when
+# this pin moves.
 giltDir: ~/.gilt/clone
 debug: false
 parallel: true

docs/sonic-config-validation.md

@@ -46,6 +46,70 @@ Tables like these are listed in `PLATFORM_DIVERGENT_TABLES` in the generator.
 They get no schema and are reported as a warning naming the reason, instead of
 producing errors about values that are correct.
 
+Where the divergence is one leaf rather than a whole table, dropping the table
+gives up too much. Those are listed in `PLATFORM_DIVERGENT_FIELDS` instead,
+which retypes just that leaf to what the platform accepts and emits the reason
+as a comment beside the generated field. The rest of the table stays validated,
+and so does the leaf — against the platform rather than against the model.
+
+| field                          | community model | what the devices run |
+|--------------------------------|-----------------|----------------------|
+| `BGP_NEIGHBOR_AF.admin_status` | enum `up`/`down`| `true`/`false`       |
+
+### Why `BGP_NEIGHBOR_AF.admin_status` is `true`/`false`
+
+This one is worth spelling out, because the community model is not merely a
+different flavour here — following it takes BGP down on the builds we run.
+
+`BGP_NEIGHBOR_AF` belongs to the unified FRR management interface, and that
+feature's own CONFIG_DB schema types the leaf as a boolean, in the same
+document where `PORT` and the interface tables are `up`/`down`:
+
+    admin_status = "true" / "false" ; Neighbor admin status
+
+— `doc/mgmt/SONiC_Design_Doc_Unified_FRR_Mgmt_Interface.md` §3.2.1.7 in
+`sonic-net/SONiC`.
+
+The consumer agrees. In `frrcfgd`, the address-family `admin_status` entries of
+`nbr_af_key_map` are bound to the token pair `['true', 'false', False]`. A
+value outside that pair does not fall back to anything: `get_command_cmn` logs
+`Input token up is neither true or false` and returns no command, so
+`neighbor <x> activate` is never issued and the address family is silently left
+inactive. A config that reads correctly produces a peering that carries no
+routes.
+
+The community YANG model says otherwise — `stypes:admin_status`, the `up`/`down`
+enumeration — and upstream resolved the contradiction in the consumer, not in
+the model: `frrcfgd` was taught to accept `up`/`down` *in addition to*
+`true`/`false`, explicitly so that existing deployments keep working
+([sonic-buildimage#21697][af-admin], merged 2025-03-12). That change is in
+community `master` and `202505`; it is **not** in `202411`, `202405` or
+`202311`.
+
+So the generator emits `true`, and this entry keeps the validator from calling
+that an error.
+
+### When this entry can go
+
+When every switch OSISM manages runs a build whose `frrcfgd` carries that
+change. Then `up`/`down` becomes the better value — it satisfies both the model
+and the consumer, and `config reload` on newer `sonic-utilities` YANG-validates
+`/etc/sonic/config_db.json` and aborts on a value outside the model.
+
+Establish it on a device, not from a release note. Enterprise builds are cut
+from community branches, but which branch a given build carries is not
+something a version number answers, and no published mapping settles it. The
+check is in the `bgp` container's `frrcfgd.py`: the `admin_status|ipv4` entry
+of `nbr_af_key_map` must name a handler function rather than the literal token
+list.
+
+Change both sides together. Emitting `up` while this entry still says
+`true`/`false` turns every generated AF row into a validation error; retyping
+the leaf while the generator still emits `true` does the same in reverse. And
+neither is safe until the fleet has moved, whatever the validator says.
+
+[af-admin]: https://github.com/sonic-net/sonic-buildimage/pull/21697
+
 Vendoring the devices' own models instead is not currently possible: there is
 no authoritative published Enterprise model set. The management-framework
 lineage in `sonic-net/sonic-mgmt-common` carries only a handful of modules, the

osism/tasks/conductor/sonic/_generated/_schemas.py

@@ -663,7 +663,10 @@ class BgpNeighborAfListRow(BaseModel):
     vrf_name: Optional[str] = None
     neighbor: Optional[str] = None
     afi_safi: Optional[str] = None
-    admin_status: Optional[Literal["up", "down"]] = None
+    # Platform divergence: the frr-mgmt-framework CONFIG_DB schema types this
+    # leaf true/false, and frrcfgd only activates the address family for those
+    # two tokens; see docs/sonic-config-validation.md
+    admin_status: Optional[Literal["true", "false"]] = None
     send_default_route: Optional[bool] = None
     default_rmap: Optional[str] = None
     max_prefix_limit: Optional[Annotated[int, Field(ge=0, le=4294967295)]] = None

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

@@ -393,6 +393,17 @@ def _warnings_for(result, table):
     return [w for w in result.warnings if table in w]
 
 
+def _bgp_neighbor_af_config(admin_status):
+    """A BGP_NEIGHBOR_AF row with the neighbor its key references, so nothing
+    but the admin_status value is under test."""
+    return {
+        "BGP_NEIGHBOR": {"default|Ethernet0": {"peer_type": "external"}},
+        "BGP_NEIGHBOR_AF": {
+            "default|Ethernet0|ipv4_unicast": {"admin_status": admin_status},
+        },
+    }
+
+
 def test_platform_divergent_table_is_not_schema_validated():
     """The vendored models are community SONiC; these devices run an
     Enterprise build that models SYSLOG_SERVER with different field names and
@@ -430,6 +441,44 @@ def test_platform_divergent_mgmt_port_accepts_the_device_value():
     assert [e for e in result.errors if e.table == "MGMT_PORT"] == [], result.errors
 
 
+def test_platform_divergent_af_admin_status_accepts_the_device_value():
+    """BGP_NEIGHBOR_AF.admin_status is true/false on the target platform: that
+    is what the frr-mgmt-framework CONFIG_DB schema specifies and the only
+    spelling frrcfgd turns into `neighbor <x> activate`. The community model
+    types the leaf as the up/down enum shared with PORT."""
+    config = _bgp_neighbor_af_config("true")
+    result = validate_config(config)
+    assert [
+        e for e in result.errors if e.table == "BGP_NEIGHBOR_AF"
+    ] == [], result.errors
+
+
+def test_platform_divergent_af_admin_status_rejects_the_yang_spelling():
+    """`up` is what the community YANG model asks for and what the platform
+    ignores: frrcfgd never activates the address family for it. The carve-out
+    keeps the field validated against the platform rather than dropping it,
+    so emitting the model's spelling here is an error."""
+    result = validate_config(_bgp_neighbor_af_config("up"))
+    assert [
+        e for e in result.errors if e.table == "BGP_NEIGHBOR_AF"
+    ] != [], result.errors
+
+
+def test_platform_divergent_field_leaves_the_rest_of_the_table_validated():
+    """Overriding one leaf must not cost the table its schema the way
+    PLATFORM_DIVERGENT_TABLES does — its other fields stay checked."""
+    config = _bgp_neighbor_af_config("true")
+    config["BGP_NEIGHBOR_AF"]["default|Ethernet0|ipv4_unicast"][
+        "send_community"
+    ] = "foobar"
+    result = validate_config(config)
+    assert any(
+        "send_community" in e.path
+        for e in result.errors
+        if e.table == "BGP_NEIGHBOR_AF"
+    ), result.errors
+
+
 def test_platform_divergent_table_drops_its_own_leafrefs():
     """SYSLOG_SERVER.vrf is a community-only field — the platform spells it
     vrf_name — so the constraint sourced from it must go with the schema."""

tools/sonic_yang_to_pydantic.py

@@ -22,6 +22,7 @@
 import re
 import subprocess
 import sys
+import textwrap
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Dict, List, Optional, Tuple
@@ -82,6 +83,23 @@
     "MGMT_PORT": ("the platform models autoneg as a boolean, not as `on`/`off`"),
 }
 
+# Individual leaves the vendored models type differently from the platform,
+# where opting the whole table out via PLATFORM_DIVERGENT_TABLES would give up
+# too much. Maps (table, leaf) to the annotation the platform actually accepts
+# plus the reason, which is emitted as a comment beside the generated field.
+#
+# Same evidentiary bar as PLATFORM_DIVERGENT_TABLES, and the same trap: our own
+# generated configs are not evidence. Each entry needs the platform's own
+# schema or its consumer, and a note on what would let it be dropped again.
+PLATFORM_DIVERGENT_FIELDS = {
+    ("BGP_NEIGHBOR_AF", "admin_status"): (
+        'Optional[Literal["true", "false"]] = None',
+        "the frr-mgmt-framework CONFIG_DB schema types this leaf true/false, "
+        "and frrcfgd only activates the address family for those two tokens; "
+        "see docs/sonic-config-validation.md",
+    ),
+}
+
 YANG_INT_BOUNDS = {
     "int8": (-(2**7), 2**7 - 1),
     "int16": (-(2**15), 2**15 - 1),
@@ -577,13 +595,32 @@ def default_for_type(default_arg: str, annotation: str) -> str:
     return repr(default_arg)
 
 
-def leaf_field_decl(leaf_stmt) -> str:
+def leaf_field_decl(leaf_stmt, table_name: Optional[str] = None) -> str:
+    field_name, alias = safe_field_name(leaf_stmt.arg)
+
+    override = (
+        PLATFORM_DIVERGENT_FIELDS.get((table_name, leaf_stmt.arg))
+        if table_name is not None
+        else None
+    )
+    if override is not None:
+        annotation, reason = override
+        if alias:
+            raise NotImplementedError(
+                f"platform-divergent field {table_name}.{leaf_stmt.arg} needs an "
+                "alias, which the override does not carry"
+            )
+        comment = "\n".join(
+            f"    # {line}"
+            for line in textwrap.wrap(f"Platform divergence: {reason}", width=74)
+        )
+        return f"{comment}\n    {field_name}: {annotation}"
+
     py = (
         yang_type_to_py(leaf_stmt.search_one("type"))
         if leaf_stmt.search_one("type")
         else PyType("Any")
     )
-    field_name, alias = safe_field_name(leaf_stmt.arg)
     mandatory = is_mandatory(leaf_stmt)
     default_stmt = leaf_stmt.search_one("default")
 
@@ -682,7 +719,7 @@ def generate_row_class(
     rows = []
     for leaf in leaves:
         if leaf.keyword == "leaf":
-            rows.append(leaf_field_decl(leaf))
+            rows.append(leaf_field_decl(leaf, table_name))
         elif leaf.keyword == "leaf-list":
             rows.append(leaf_list_field_decl(leaf, table_name))
     if not rows: