FabricContext: expose platform_type, retain switch records, fail closed (#399, #400)

[allenrobel]

⚠️ Postponed — please do not merge

Labeled Postponed on 2026-07-16. This PR has one approval and is otherwise ready, so this note exists to stop a second approver from merging it past an unresolved design question.

Why. The PlatformTypeEnum added here (plugins/module_utils/enums.py) duplicates PlatformType, which already exists in develop at plugins/module_utils/models/manage_switches/enums.py — same values, except the existing one also has SONIC. That omission is a live defect in this PR: FabricContext.get_platform_type() resolves via PlatformTypeEnum(raw) inside a try/except ValueError, so a SONiC switch silently returns None rather than its platform. Two enums one token apart in the name (PlatformType / PlatformTypeEnum) is also a trap for the next reader.

Proposed resolution. Promote PlatformType up to module_utils/enums.py (per CLAUDE.md, the home for "Enums commonly used by most module utilities" — both manage_switches and fabric_context now need it) and have FabricContext use it, dropping PlatformTypeEnum. A move + re-export keeps every existing import site working, so no in-flight PR needs a rebase.

Blocked on. @AKDRG's read, being discussed on #405 Resolved — @AKDRG implemented the proposed promotion in #405 (approved 2026-07-22); see the 2026-07-23 update below. Original discussion: #405 (comment)

The existing enums are deliberately endpoint-scoped (PlatformType = AddSwitches POST; ShallowDiscoveryPlatformType split out because that endpoint excludes apic). This PR's use is a third context — the GET switches response. If that response set can legitimately diverge from the AddSwitches request set, sharing one enum is the wrong call and a distinct, properly named read-side enum stays — with SONIC added. The SONIC gap needs fixing either way.

Nothing else in this PR is affected: the #399 fidelity fix, the #400 fail-closed hardening, and the _Sentinel change all stand on their own. Remove the Postponed label once the enum question is settled.

Update 2026-07-23 — design question settled; this is now a pure merge-ordering hold on #405. The discussion on #405 resolved in favor of the promotion: #405 (approved 2026-07-22) moves PlatformType up to plugins/module_utils/enums.py.

Hold this PR until #405 merges, then rebase with the following:

• Drop this PR's PlatformTypeEnum from enums.py and point FabricContext.get_platform_type() at the promoted
  PlatformType (which includes SONIC, closing the silent-None defect described above).
• Remove the Postponed label.

Sequencing note for reviewers: this PR should land before the upcoming IOS-XE interface union wave (ethernet, port-channel, SVI, sub-interface — following the #403 loopback pattern). Those PRs plan to make network_os_type optional via the FabricContext.get_platform_type() fallback added here.

Related Issue(s)

Closes #399
Closes #400

Proposed Changes

Three related changes to FabricContext (plugins/module_utils/fabric_context.py), plus a reusable enum:

• feat — expose switch platform type. Retain the raw switch records from GET /api/v1/manage/fabrics/{fabric}/switches and add get_platform_type(switch_ip) -> PlatformTypeEnum | None (reads the nested additionalData.platformType), plus a switches property exposing the retained records. New PlatformTypeEnum (apic/ios-xe/ios-xr/nx-os/other) in enums.py. This lets callers select a platform-appropriate feature model (e.g. loopback vs iosXeLoopback) without exposing the value to users — the switch-derived networkOSType the interface-granularity design calls for.
• fix FabricContext: nonexistent fabric reports "switch not found" instead of "fabric not found" (nd_fabric_update_group, nd_manage_l3out) #399 — fabric-not-found fidelity. The switches endpoint returns 404 when the parent fabric is absent; _query_get swallowed that into an empty map, so get_switch_id surfaced a misleading "switch not found". _load_switch_maps now confirms a 404 against fabric_summary and raises the fabric-level "fabric not found" message (shared with validate_for_mutation via _fabric_not_found_message). Not an ND deviation, so no TODO(X.Y.Z)/vault marker.
• hardening FabricContext: harden fabric_summary to reject payloads carrying an embedded 'code' error key (fail closed) #400 — fail closed. fabric_summary now rejects a 200 body carrying an embedded {"code": N, ...} error key instead of accepting it as a valid summary (which would let every validate_for_mutation check default open). Kept narrow (summary only; not pushed into _query_get) to avoid changing behavior for all 13 FabricContext consumers.

Incidentally replaces the _NOT_FETCHED = object() sentinel on _fabric_summary with a typed one — a module-private single-member _Sentinel enum, spelled Literal[_Sentinel.UNSET] in the union — so the property is correctly typed dict | None (clears a pre-existing mypy/Pylance object-return smell on the touched code). _fabric_summary needs a tri-state because None is a load-bearing value meaning "the fabric does not exist"; object() couldn't be narrowed by a type checker, whereas the enum can. Keeps the state in one field rather than two that must be kept in sync, so invalidate() is a single assignment. Per @akinross review. No behavior change.

Test Notes

• Unit: +5 tests (embedded-code hardening, platform_type/switches, switches-404 → fabric-not-found, plus two for the _Sentinel tri-state: a fetched-but-absent None summary is cached so fabric_exists() doesn't re-query, and invalidate() clears the cached None rather than pinning it), new fixtures for each.
• The two _Sentinel tests were verified to fail against a deliberately mutated implementation (invalidate() resetting to None; sentinel dropped for None-means-unset), confirming they guard the invariant rather than passing vacuously.
• Full module_utils unit suite green: 3036 passed via ndpytest.
• black, isort, pylint, mypy clean on the changed files (nd-dev container).
• ND behavior (404 on both fabric endpoints for a missing fabric; platformType enum values) verified against a live ND 4.2.1 lab, as documented in FabricContext: nonexistent fabric reports "switch not found" instead of "fabric not found" (nd_fabric_update_group, nd_manage_l3out) #399/FabricContext: harden fabric_summary to reject payloads carrying an embedded 'code' error key (fail closed) #400.

Cisco Nexus Dashboard Version

4.2.1

Related ND API Resource Category

• analyze
• infra
• manage
• onemanage
• other

Checklist

• Latest commit is rebased from develop with merge conflicts resolved
• New or updates to documentation has been made accordingly
• Assigned the proper reviewers

[Copilot]

Pull request overview

This PR enhances FabricContext to expose cached switch inventory details (including platform type), improves error fidelity when a fabric is missing, and hardens fabric_summary to fail closed on embedded-error payloads. These changes strengthen orchestrator pre-flight behavior and enable platform-aware feature selection without surfacing platformType to end users.

Changes:

• Add PlatformTypeEnum, retain raw switch records in FabricContext, and expose switches + get_platform_type(switch_ip).
• Fix missing-fabric error fidelity when the switches endpoint returns 404 by raising a fabric-level “not found” message.
• Harden fabric_summary to reject 200 payloads containing an embedded code error key, with unit tests/fixtures added.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
plugins/module_utils/fabric_context.py	Adds platform-type lookup + switch record retention; improves missing-fabric error fidelity; hardens summary handling.
plugins/module_utils/enums.py	Introduces PlatformTypeEnum for additionalData.platformType.
tests/unit/module_utils/test_fabric_context.py	Adds unit tests covering embedded-error summary handling, platformType lookup, and switches-404 → fabric-not-found behavior.
tests/unit/module_utils/fixtures/fixture_data/test_fabric_context.json	Adds fixtures for the new tests (embedded code payload, platformType values, switches 404 + summary 404).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

plugins/module_utils/fabric_context.py:150

• fabric_exists() now calls fabric_summary(), which can raise RuntimeError when the summary payload contains an embedded code error key. The docstring currently says this method raises nothing, which is no longer accurate and can mislead callers/users of this helper.

    def fabric_exists(self) -> bool:
        """
        # Summary

        Check whether the fabric exists (on any ND node in the cluster).

        ## Raises

        None
        """
        return self.fabric_summary is not None

plugins/module_utils/fabric_context.py:287

• switch_map can now raise a fabric-level RuntimeError for a nonexistent fabric (via _load_switch_maps()), but its docstring only documents API-query failures. Documenting the new failure mode makes the public accessor’s contract accurate.

    def switch_map(self) -> dict[str, str]:
        """
        # Summary

        Return a cached mapping of `fabricManagementIp` to `switchId` for all switches in the fabric.

        Fetches all switches from the ND Manage Switches API on first access and caches the result.

        ## Raises

        ### RuntimeError

        - If the switches API query fails.
        """
        self._load_switch_maps()

[allenrobel]

@copilot-pull-request-reviewer Thanks — both low-confidence suggestions from the latest review are correct, and I've addressed them (these were suppressed as low-confidence, so there are no inline threads to reply on).

Both ## Raises sections were stale after this PR's new RuntimeError paths:

• fabric_exists() — now raises via fabric_summary on an embedded code error payload; docstring said "None".
• switch_map — now raises the fabric-not-found error via _load_switch_maps; docstring documented only the API-query failure.

I also applied the same fix to three sibling methods that share the identical defect but weren't flagged, so the docstrings stay consistent:

• fabric_is_local and fabric_is_deployment_frozen (same fabric_summary raise path as fabric_exists).
• switch_map_by_id (same _load_switch_maps raise path as switch_map).

Docs-only, no behavior change; fabric_context unit tests remain green. Fixed in commit "FabricContext: document new RuntimeError paths in ## Raises (Copilot low-confidence)".

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

[akinross]

slight consideration but code looks good, if not something you think we should consider it is approved from my end