Skip to content

refactor(controller): remove transitional fallback and complete Phase 3 cleanup (#897, #914, #917, #918) - #953

Open
oboehmer wants to merge 4 commits into
mainfrom
refactor/controller-phase3-cleanup
Open

oboehmer wants to merge 4 commits into
mainfrom
refactor/controller-phase3-cleanup

Conversation

@oboehmer

@oboehmer oboehmer commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Complete Phase 3 controller cleanup by removing transitional fallbacks, deleting the deprecated nac_test.utils.controller shim, updating test suites to use unified context fixtures, and aligning documentation with the SSOT controller registry architecture.

Closes

Related Issue(s)

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring / Technical debt (internal improvements with no user-facing changes)
  • Documentation update
  • Chore (build process, CI, tooling, dependencies)
  • Other (please describe):

Test Framework Affected

  • PyATS
  • Robot Framework
  • Both
  • N/A (not test-framework specific)

Network as Code (NaC) Architecture Affected

  • ACI (APIC)
  • NDO (Nexus Dashboard Orchestrator)
  • NDFC / VXLAN-EVPN (Nexus Dashboard Fabric Controller)
  • Catalyst SD-WAN (SDWAN Manager / vManage)
  • Catalyst Center (DNA Center)
  • ISE (Identity Services Engine)
  • FMC (Firepower Management Center)
  • Meraki (Cloud-managed)
  • NX-OS (Nexus Direct-to-Device)
  • IOS-XE (Direct-to-Device)
  • IOS-XR (Direct-to-Device)
  • Hyperfabric
  • All architectures
  • N/A (architecture-agnostic)

Platform Tested

nac-test supports macOS and Linux only

  • macOS (version tested: Darwin 25.3.0)
  • Linux (distro/version tested: )

Key Changes

  • Removed transitional fallback from get_controller_context(), enforcing that controller identity must be resolved by the orchestrator and passed via NAC_TEST_CONTROLLER_CONTEXT.
  • Deleted deprecated nac_test/utils/controller.py shim.
  • Injected NAC_TEST_CONTROLLER_CONTEXT in preflight_auth_check() for auth adapters (e.g., SDWANManagerAuth) reading get_controller_context().
  • Added resolve_and_inject_context and inject_context helper fixtures in tests/conftest.py.
  • Cleaned up redundant per-test environment unsetting and updated empty-environment tests with fixture notes.
  • Updated dev-docs/PRD_AND_ARCHITECTURE.md to reflect resolve_controller() -> ControllerContext and SSOT controller resolution.

Testing Done

  • Unit tests added/updated
  • Integration tests performed
  • Manual testing performed:
    • PyATS tests executed successfully
    • Robot Framework tests executed successfully
    • D2D/SSH tests executed successfully (if applicable)
    • HTML reports generated correctly
  • All existing tests pass (pytest / pre-commit run -a)

Test Commands Used

.venv/bin/pre-commit run --all-files
.venv/bin/python -m pytest tests/unit tests/pyats_core -n auto --dist loadscope -q

Checklist

  • Code follows project style guidelines (pre-commit run -a passes)
  • Self-review of code completed
  • Code is commented where necessary (especially complex logic)
  • Documentation updated (if applicable)
  • No new warnings introduced
  • Changes work on both macOS and Linux
  • CHANGELOG.md updated (if applicable)

Additional Notes

Paired with nac-test-pyats-common branch refactor/controller-phase3-cleanup.

… 3 cleanup (#897, #914, #917, #918)

- Remove transitional fallback from get_controller_context()
- Delete deprecated nac_test/utils/controller.py shim
- Clean up test suite to use resolve_and_inject_context and inject_context fixtures
- Update dev-docs/PRD_AND_ARCHITECTURE.md to reflect SSOT controller resolution
Ensure controller context is present in os.environ for auth adapters
(such as SDWANManagerAuth) that read get_controller_context() during
preflight validation.
NACTestBase.setup() requires NAC_TEST_CONTROLLER_CONTEXT since the
Phase 3 fallback removal. Patching NACTestBase.setup masked this on
Python 3.12/3.13 but not on 3.10, where pyATS binds @aetest.setup at
import time and super().setup() bypasses the class-level patch.

Refs #954
@oboehmer
oboehmer marked this pull request as ready for review September 14, 2026 10:08
@oboehmer
oboehmer requested a review from aitestino September 14, 2026 10:08
oboehmer added a commit to netascode/nac-test-pyats-common that referenced this pull request Sep 14, 2026
…tract

Phase 3 removes the transitional fallback in nac-test's
get_controller_context(), so SDWANManagerAuth.get_auth() now hard-fails
outside an orchestrated run. The docs and tests did not reflect that.

- Document get_auth() as orchestrator-only in both the module and method
  docstrings: the controller context is resolved by nac-test via
  resolve_controller() and passed through NAC_TEST_CONTROLLER_CONTEXT,
  so exporting SDWAN_* credentials alone is no longer sufficient.
- List NAC_TEST_CONTROLLER_CONTEXT as a requirement and add the
  missing-context ValueError to Raises:.
- Remove the get_auth() doctest examples. They demonstrated standalone
  credential-only usage, which is no longer a valid use case and now
  raises. Doctests are not collected, so nothing caught the drift.
- Add TestControllerContextRequired, which deliberately does not mock
  get_controller_context(). It is the only test that distinguishes a
  Phase 3 nac-test from a Phase 2 one: against Phase 2 the deprecated
  detect_controller_type() fallback resolves SDWAN from the credentials
  and get_auth() attempts a real connection instead of raising.
  Expected to fail until the nac-test pin is bumped.
- Correct stale references in dev-docs/PRD_AND_ARCHITECTURE.md: the
  deleted nac_test/utils/controller.py, a hardcoded absolute path, and
  two flow diagrams that attributed the UNKNOWN fallback to
  get_controller_context() rather than to the except in IOSXETestBase.

Refs netascode/nac-test#897
Depends on netascode/nac-test#953
@oboehmer oboehmer added tech-debt General technical debt requiring refactoring refactor Code refactoring without changing functionality labels Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring without changing functionality tech-debt General technical debt requiring refactoring

Projects

None yet

1 participant