diff --git a/dev-docs/PRD_AND_ARCHITECTURE.md b/dev-docs/PRD_AND_ARCHITECTURE.md index 28e638e6..7ed02497 100644 --- a/dev-docs/PRD_AND_ARCHITECTURE.md +++ b/dev-docs/PRD_AND_ARCHITECTURE.md @@ -5960,14 +5960,14 @@ Example: `IOSXE_USERNAME` and `IOSXE_PASSWORD` could be used for: │ IOSXE_PASSWORD=device-pass │ │ │ │ Detection Result: │ -│ detect_controller_type() → "sdwan" │ +│ resolve_controller() → ControllerContext(controller_type="SDWAN")│ │ → TestTypeResolver uses BASE_CLASS_MAPPING["SDWANTestBase"] │ │ → Device SSH uses IOSXE_USERNAME/IOSXE_PASSWORD │ └────────────────────────────────────────────────────────────────────┘ ``` **What happens during D2D test execution:** -1. Framework detects `CONTROLLER_TYPE=SDWAN` from controller credentials +1. Framework detects `controller_context.controller_type == "SDWAN"` from controller credentials 2. Framework loads `SDWANDeviceResolver` for device inventory resolution 3. Tests connect to devices via SSH using `IOSXE_*` credentials 4. Controller credentials are NOT used for connection (D2D tests bypass controller) @@ -5978,11 +5978,8 @@ The detected controller type informs test categorization: ```python # In orchestrator.py -controller_type = detect_controller_type() -if controller_type: - logger.info(f"Detected controller type: {controller_type}") -else: - logger.warning("Could not auto-detect controller type from environment") +controller_context = resolve_controller() +logger.info(f"Detected controller type: {controller_context.controller_type}") ``` #### Usage Examples diff --git a/nac_test/core/controller.py b/nac_test/core/controller.py index d6867e0a..9696bd92 100644 --- a/nac_test/core/controller.py +++ b/nac_test/core/controller.py @@ -18,7 +18,6 @@ import logging import os -import warnings from collections.abc import Mapping, Sequence from dataclasses import dataclass from types import MappingProxyType @@ -272,10 +271,6 @@ class ControllerConfig: ), } -# Module-level cache for the credential set that was matched during detection. -# Populated by detect_controller_type(), consumed by get_matched_credential_set(). -_matched_credential_sets: dict[str, CredentialSet] = {} - class ResolutionError(NacTestError): """Base for controller resolution failures.""" @@ -313,10 +308,6 @@ def resolve_controller() -> ControllerContext: :class:`ResolutionError` subclass on failure. The caller decides how to handle failures — this function never calls ``sys.exit()``. - Side-effects: - * Populates ``_matched_credential_sets`` (same as the legacy - ``detect_controller_type()``). - Returns: ControllerContext with ``controller_type`` and ``auth_method``. @@ -340,7 +331,6 @@ def resolve_controller() -> ControllerContext: # Exactly one complete set — success controller_type = next(iter(complete)) matched_cred_set = complete[controller_type] - _matched_credential_sets[controller_type] = matched_cred_set ctx = ControllerContext( controller_type=controller_type, @@ -364,42 +354,30 @@ def get_controller_context() -> ControllerContext: ``PyATSOrchestrator``, which serializes it to ``NAC_TEST_CONTROLLER_CONTEXT`` before launching subprocesses. - **Primary path (subprocess):** Deserializes from ``NAC_TEST_CONTROLLER_CONTEXT`` - environment variable set by ``PyATSOrchestrator``. - - **Fallback (transitional):** If the env var is absent, falls back to - ``detect_controller_type()`` for backwards compatibility. This fallback - will be removed in Phase 3 once all consumers are migrated. + Deserializes from ``NAC_TEST_CONTROLLER_CONTEXT`` environment variable + set by ``PyATSOrchestrator``. Returns: ControllerContext with controller_type and auth_method. Raises: - ValueError: If no controller credentials are found (via fallback path). + ValueError: If ``NAC_TEST_CONTROLLER_CONTEXT`` is not set or invalid. """ raw = os.environ.get(ENV_CONTROLLER_CONTEXT) - if raw: - return ControllerContext.from_json(raw) - - # --- Transitional fallback (remove in Phase 3) ----------------------- - logging.getLogger(__name__).info( - "NAC_TEST_CONTROLLER_CONTEXT not set — falling back to " - "detect_controller_type(). This fallback will be removed in a " - "future release." - ) - - controller_type = detect_controller_type() - return ControllerContext( - controller_type=controller_type, - auth_method=_infer_auth_method(controller_type), - ) + if not raw: + raise ValueError( + f"Environment variable {ENV_CONTROLLER_CONTEXT} is not set. " + "Controller context must be resolved by the orchestrator via " + "resolve_controller() and passed to subprocesses." + ) + return ControllerContext.from_json(raw) def format_resolution_error(error: ResolutionError) -> str: """Format a :class:`ResolutionError` into a user-facing message. - Re-uses the existing detailed error formatters so that CLI output - stays identical to the legacy ``detect_controller_type()`` path. + Uses the detailed error formatters for multiple, incomplete, or missing + credentials. """ if isinstance(error, MultipleControllersFound): return _format_multiple_credentials_error(error.controllers) @@ -679,85 +657,6 @@ def _set_var_count(cs: CredentialSet) -> int: return values -def detect_controller_type() -> ControllerTypeKey: - """Detect the controller type based on environment variables. - - .. deprecated:: - This function is retained for backwards compatibility with external - packages (e.g., ``nac-test-pyats-common``) that have not yet migrated - to :func:`resolve_controller`. New code should use ``resolve_controller()`` - directly and handle :class:`ResolutionError` subtypes. This function - will be removed once all consumers have migrated. - - This function examines environment variables to determine which network controller - architecture is being targeted. It ensures exactly one controller type has credentials - configured to prevent ambiguous test contexts. - - Controller credentials are required for ALL test types: - - API tests: Use credentials directly for controller authentication - - D2D tests: Use controller type to determine device resolution logic - - Returns: - The detected controller type (e.g., "ACI", "SDWAN", "CC", "MERAKI", "FMC", "ISE"). - - Raises: - ValueError: If no controller credentials are found, multiple controllers are - configured, or credentials are incomplete. - - Example: - >>> os.environ.update({"ACI_URL": "https://apic.local", - ... "ACI_USERNAME": "admin", - ... "ACI_PASSWORD": "pass"}) - >>> controller = detect_controller_type() - >>> print(controller) - "ACI" - - Note: - This function delegates to :func:`resolve_controller` and converts typed - exceptions to ``ValueError`` for backwards compatibility with existing callers. - """ - warnings.warn( - "detect_controller_type() is deprecated; use resolve_controller() instead.", - DeprecationWarning, - stacklevel=2, - ) - try: - ctx = resolve_controller() - return ctx.controller_type - except ResolutionError as e: - raise ValueError(format_resolution_error(e)) from e - - -def get_matched_credential_set(controller_type: str) -> CredentialSet | None: - """Get the credential set that was matched during controller detection. - - .. deprecated:: - This function is a transitional API for ``nac-test-pyats-common`` auth - adapters. It will be removed in Phase 3 once auth adapters migrate to - using ``get_controller_context().auth_method`` directly. New code should - not use this function. - - Returns the CredentialSet that satisfied detection for the given controller - type. This is populated by detect_controller_type() / resolve_controller() - and is intended for use by auth adapters in nac-test-pyats-common to - determine which authentication mechanism to use. - - Args: - controller_type: The controller type key (e.g., "SDWAN", "ACI"). - - Returns: - The matched CredentialSet, or None if detection has not been called - or the controller type was not detected. - """ - warnings.warn( - "get_matched_credential_set() is deprecated; use " - "get_controller_context().auth_method instead.", - DeprecationWarning, - stacklevel=2, - ) - return _matched_credential_sets.get(controller_type) - - def _find_credential_sets() -> tuple[ dict[ControllerTypeKey, CredentialSet], list[ControllerTypeKey], @@ -804,23 +703,6 @@ def _find_credential_sets() -> tuple[ return complete, partial -def _infer_auth_method(controller_type: str) -> AuthMethod: - """Infer auth_method by scanning env vars for a controller type. - - Used only in the transitional fallback path of - ``get_controller_context()`` when ``NAC_TEST_CONTROLLER_CONTEXT`` - is absent. Mirrors the logic of ``_find_credential_sets()`` but - returns only the auth_method string. - """ - config = CONTROLLER_REGISTRY.get(controller_type) - if config is None: - return AuthMethod.SESSION - for cred_set in config.credential_sets: - if all(is_env_var_set(v) for v in cred_set.env_vars): - return cred_set.auth_method - return AuthMethod.SESSION - - def _format_incomplete_credentials_error(partial_controllers: Sequence[str]) -> str: """Format error message for incomplete controller credentials. diff --git a/nac_test/core/controller_auth.py b/nac_test/core/controller_auth.py index a93ccf05..3e3a5085 100644 --- a/nac_test/core/controller_auth.py +++ b/nac_test/core/controller_auth.py @@ -11,6 +11,7 @@ """ import logging +import os import sys from collections.abc import Callable from dataclasses import dataclass @@ -19,6 +20,7 @@ from nac_test.core.auth_cache import AuthCache from nac_test.core.controller import ( CONTROLLER_REGISTRY, + ENV_CONTROLLER_CONTEXT, get_controller_url, get_display_name, ) @@ -142,6 +144,10 @@ def preflight_auth_check(ctx: ControllerContext) -> AuthCheckResult: detail="Pre-flight check skipped (no auth adapter available)", ) + # Ensure controller context env var is set for auth adapters (e.g., SDWANManagerAuth) + # that inspect get_controller_context() during get_auth(). + os.environ[ENV_CONTROLLER_CONTEXT] = ctx.to_json() + # Invalidate any stale cached token so we validate the current credentials. # Best-effort: a failure here must never block test execution. config = CONTROLLER_REGISTRY.get(controller_type) diff --git a/nac_test/pyats_core/common/base_test.py b/nac_test/pyats_core/common/base_test.py index 5dde72e1..ca5d3b52 100644 --- a/nac_test/pyats_core/common/base_test.py +++ b/nac_test/pyats_core/common/base_test.py @@ -172,10 +172,9 @@ def setup(self) -> None: self.data_model = self.load_data_model() # Get controller context from environment - # In normal operation, CombinedOrchestrator resolves the controller and - # passes it via NAC_TEST_CONTROLLER_CONTEXT env var. The accessor - # get_controller_context() reads this, with a fallback to env var scan - # for direct pyats invocation or legacy compatibility. + # CombinedOrchestrator resolves the controller and passes it via + # the NAC_TEST_CONTROLLER_CONTEXT env var. The accessor + # get_controller_context() deserializes this context. try: ctx = get_controller_context() except (ValueError, KeyError) as e: diff --git a/nac_test/utils/controller.py b/nac_test/utils/controller.py deleted file mode 100644 index 9611d326..00000000 --- a/nac_test/utils/controller.py +++ /dev/null @@ -1,18 +0,0 @@ -# SPDX-License-Identifier: MPL-2.0 -# Copyright (c) 2025 Daniel Schmidt - -"""Bridge-release compatibility shim. - -Re-exports only the controller symbols actually used by ``nac-test-pyats-common``: -- detect_controller_type (iosxe/test_base.py) -- get_matched_credential_set (sdwan/auth.py) - -This shim exists so that ``nac-test-pyats-common`` continues to work during the -transition window. It will be removed after all consumers have migrated to -``nac_test.core.controller`` (Phase 3 of the controller-resolution refactor). -""" - -from nac_test.core.controller import ( # noqa: F401 - detect_controller_type, - get_matched_credential_set, -) diff --git a/tests/conftest.py b/tests/conftest.py index e5b011ab..feea3384 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,7 +18,7 @@ import pytest from nac_test.core.constants import ENV_CONTROLLER_CONTEXT -from nac_test.core.controller import CONTROLLER_REGISTRY +from nac_test.core.controller import CONTROLLER_REGISTRY, resolve_controller from nac_test.core.types import AuthMethod, ControllerContext from tests.e2e.mocks.mock_server import MockAPIServer @@ -68,11 +68,6 @@ def clean_controller_env(monkeypatch: pytest.MonkeyPatch) -> None: # Clear serialized controller context from previous tests monkeypatch.delenv(ENV_CONTROLLER_CONTEXT, raising=False) - # Clear module-level credential cache to prevent cross-test pollution - from nac_test.core import controller - - controller._matched_credential_sets.clear() - @pytest.fixture(scope="session", autouse=True) def bypass_proxy_for_localhost() -> Generator[None, None, None]: @@ -193,3 +188,21 @@ def cc_context() -> ControllerContext: def iosxe_context() -> ControllerContext: """Pre-built ControllerContext for IOS-XE with session auth.""" return ControllerContext(controller_type="IOSXE", auth_method=AuthMethod.SESSION) + + +# ============================================================================= +# Context injection test helpers +# ============================================================================= + + +def resolve_and_inject_context(monkeypatch: pytest.MonkeyPatch) -> ControllerContext: + """Resolve controller from current environment and inject into ENV_CONTROLLER_CONTEXT. + Designed for happy-path tests to avoid DRY repetition.""" + ctx = resolve_controller() + monkeypatch.setenv(ENV_CONTROLLER_CONTEXT, ctx.to_json()) + return ctx + + +def inject_context(monkeypatch: pytest.MonkeyPatch, ctx: ControllerContext) -> None: + """Inject a pre-built ControllerContext into ENV_CONTROLLER_CONTEXT.""" + monkeypatch.setenv(ENV_CONTROLLER_CONTEXT, ctx.to_json()) diff --git a/tests/integration/test_cli_aci_validation.py b/tests/integration/test_cli_aci_validation.py index 9dad95e2..5a9c299a 100644 --- a/tests/integration/test_cli_aci_validation.py +++ b/tests/integration/test_cli_aci_validation.py @@ -77,12 +77,12 @@ def test_cli_validation_triggers_when_aci_url_set_and_no_defaults( def test_cli_validation_passes_when_aci_url_not_set( self, minimal_test_env: dict[str, Path], - monkeypatch: pytest.MonkeyPatch, ) -> None: - """CLI should skip ACI validation when ACI_URL is not set.""" - # Ensure ACI_URL is not set - monkeypatch.delenv("ACI_URL", raising=False) + """CLI should skip ACI validation when ACI_URL is not set. + Relies on the global autouse ``clean_controller_env`` fixture in + ``tests/conftest.py`` ensuring ACI_URL is unset. + """ # We need to mock the DataMerger and orchestrator since we don't have full environment with ( patch("nac_test.cli.main.DataMerger") as mock_merger, diff --git a/tests/integration/test_controller_detection_integration.py b/tests/integration/test_controller_detection_integration.py index ed8fc18b..bf8f6722 100644 --- a/tests/integration/test_controller_detection_integration.py +++ b/tests/integration/test_controller_detection_integration.py @@ -7,7 +7,7 @@ import pytest -from nac_test.core.controller import detect_controller_type +from nac_test.core.controller import resolve_controller from nac_test.pyats_core.orchestrator import PyATSOrchestrator @@ -65,7 +65,7 @@ def test_controller_switch_scenario(self, monkeypatch: pytest.MonkeyPatch) -> No monkeypatch.setenv("ACI_USERNAME", "admin") monkeypatch.setenv("ACI_PASSWORD", "password") - assert detect_controller_type() == "ACI" + assert resolve_controller().controller_type == "ACI" # Clear ACI and switch to FMC monkeypatch.delenv("ACI_URL") @@ -76,7 +76,7 @@ def test_controller_switch_scenario(self, monkeypatch: pytest.MonkeyPatch) -> No monkeypatch.setenv("FMC_USERNAME", "admin") monkeypatch.setenv("FMC_PASSWORD", "password") - assert detect_controller_type() == "FMC" + assert resolve_controller().controller_type == "FMC" # Clear FMC and switch to ISE monkeypatch.delenv("FMC_URL") @@ -87,4 +87,4 @@ def test_controller_switch_scenario(self, monkeypatch: pytest.MonkeyPatch) -> No monkeypatch.setenv("ISE_USERNAME", "admin") monkeypatch.setenv("ISE_PASSWORD", "password") - assert detect_controller_type() == "ISE" + assert resolve_controller().controller_type == "ISE" diff --git a/tests/pyats_core/common/test_base_test_controller_detection.py b/tests/pyats_core/common/test_base_test_controller_detection.py index e6cdd44a..2761a5f6 100644 --- a/tests/pyats_core/common/test_base_test_controller_detection.py +++ b/tests/pyats_core/common/test_base_test_controller_detection.py @@ -14,6 +14,7 @@ from nac_test.core.constants import ENV_CONTROLLER_CONTEXT from nac_test.core.types import AuthMethod, ControllerContext from nac_test.pyats_core.common.base_test import NACTestBase +from tests.conftest import inject_context, resolve_and_inject_context @pytest.fixture @@ -33,10 +34,11 @@ class TestBaseTestControllerDetection: def test_base_test_detects_controller_on_setup( self, monkeypatch: pytest.MonkeyPatch, setup_test_data_file_env: Path ) -> None: - """Test that NACTestBase detects controller type during setup.""" + """Test that NACTestBase uses resolved controller context during setup.""" monkeypatch.setenv("ACI_URL", "https://apic.example.com") monkeypatch.setenv("ACI_USERNAME", "admin") monkeypatch.setenv("ACI_PASSWORD", "password") + resolve_and_inject_context(monkeypatch) class TestClass(NACTestBase): @aetest.test # type: ignore[misc] @@ -65,11 +67,10 @@ def test_base_test_connection_params_populated_for_iosxe( self, monkeypatch: pytest.MonkeyPatch, setup_test_data_file_env: Path ) -> None: """connection_params resolves for IOSXE too, now that kinds are populated.""" - for env_var in ["ACI_URL", "SDWAN_URL", "CC_URL"]: - monkeypatch.delenv(env_var, raising=False) monkeypatch.setenv("IOSXE_URL", "10.0.0.1") monkeypatch.setenv("IOSXE_USERNAME", "admin") monkeypatch.setenv("IOSXE_PASSWORD", "password") + resolve_and_inject_context(monkeypatch) class TestClass(NACTestBase): @aetest.test # type: ignore[misc] @@ -91,21 +92,13 @@ def test_method(self) -> None: } def test_base_test_fails_setup_on_detection_error( - self, monkeypatch: pytest.MonkeyPatch, setup_test_data_file_env: Path + self, setup_test_data_file_env: Path ) -> None: - """Test that NACTestBase fails setup when controller detection fails.""" - for env_var in [ - "ACI_URL", - "ACI_USERNAME", - "ACI_PASSWORD", - "SDWAN_URL", - "SDWAN_USERNAME", - "SDWAN_PASSWORD", - "CC_URL", - "CC_USERNAME", - "CC_PASSWORD", - ]: - monkeypatch.delenv(env_var, raising=False) + """Test that NACTestBase fails setup when controller context is missing. + + Relies on the global autouse ``clean_controller_env`` fixture in + ``tests/conftest.py`` ensuring ``ENV_CONTROLLER_CONTEXT`` is unset. + """ class TestClass(NACTestBase): @aetest.test # type: ignore[misc] @@ -120,16 +113,17 @@ def test_method(self) -> None: with pytest.raises(ValueError) as exc_info: test_instance.setup() - assert "No controller credentials found" in str(exc_info.value) + assert ENV_CONTROLLER_CONTEXT in str(exc_info.value) def test_base_test_no_longer_uses_controller_type_env_var( self, monkeypatch: pytest.MonkeyPatch, setup_test_data_file_env: Path ) -> None: - """Test that NACTestBase ignores CONTROLLER_TYPE environment variable.""" + """Test that NACTestBase uses resolved context and ignores CONTROLLER_TYPE.""" monkeypatch.setenv("SDWAN_URL", "https://vmanage.example.com") monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") monkeypatch.setenv("CONTROLLER_TYPE", "ACI") + resolve_and_inject_context(monkeypatch) class TestClass(NACTestBase): @aetest.test # type: ignore[misc] @@ -146,39 +140,13 @@ def test_method(self) -> None: assert test_instance.controller_type == "SDWAN" assert test_instance.controller_url == "https://vmanage.example.com" - def test_base_test_handles_multiple_controllers_error( - self, monkeypatch: pytest.MonkeyPatch, setup_test_data_file_env: Path - ) -> None: - """Test that NACTestBase handles multiple controller credentials error during setup.""" - monkeypatch.setenv("ACI_URL", "https://apic.example.com") - monkeypatch.setenv("ACI_USERNAME", "admin") - monkeypatch.setenv("ACI_PASSWORD", "password") - monkeypatch.setenv("CC_URL", "https://cc.example.com") - monkeypatch.setenv("CC_USERNAME", "admin") - monkeypatch.setenv("CC_PASSWORD", "password") - - class TestClass(NACTestBase): - @aetest.test # type: ignore[misc] - def test_method(self) -> None: - pass - - test_instance = TestClass() - - with patch.object( - test_instance, "load_data_model", return_value={"test": "data"} - ): - with pytest.raises(ValueError) as exc_info: - test_instance.setup() - - assert "Multiple controller credentials detected" in str(exc_info.value) - def test_base_test_uses_serialized_controller_context( self, monkeypatch: pytest.MonkeyPatch, setup_test_data_file_env: Path ) -> None: """Test that NACTestBase uses NAC_TEST_CONTROLLER_CONTEXT when present (primary path).""" # Set serialized context (primary path) AND the underlying env vars ctx = ControllerContext(controller_type="SDWAN", auth_method=AuthMethod.TOKEN) - monkeypatch.setenv(ENV_CONTROLLER_CONTEXT, ctx.to_json()) + inject_context(monkeypatch, ctx) # Need SDWAN env vars for get_controller_url() and get_connection_params() monkeypatch.setenv("SDWAN_URL", "https://vmanage.example.com") monkeypatch.setenv("SDWAN_API_TOKEN", "test-token-value") diff --git a/tests/pyats_core/common/test_base_test_result_collector.py b/tests/pyats_core/common/test_base_test_result_collector.py index 2f3a23f2..5f582cae 100644 --- a/tests/pyats_core/common/test_base_test_result_collector.py +++ b/tests/pyats_core/common/test_base_test_result_collector.py @@ -10,6 +10,7 @@ from pyats import aetest from nac_test.pyats_core.common.base_test import NACTestBase +from tests.conftest import resolve_and_inject_context class TestResultCollectorInitialization: @@ -22,6 +23,7 @@ def test_falls_back_to_cwd_when_data_file_missing( monkeypatch.setenv("ACI_URL", "https://apic.example.com") monkeypatch.setenv("ACI_USERNAME", "admin") monkeypatch.setenv("ACI_PASSWORD", "password") + resolve_and_inject_context(monkeypatch) # Point to non-existent file to trigger fallback monkeypatch.setenv( diff --git a/tests/unit/cli/validators/test_aci_defaults.py b/tests/unit/cli/validators/test_aci_defaults.py index 393721ef..8a2aaeb0 100644 --- a/tests/unit/cli/validators/test_aci_defaults.py +++ b/tests/unit/cli/validators/test_aci_defaults.py @@ -24,12 +24,12 @@ class TestValidateAciDefaults: mistake of forgetting to include -d ./defaults/ in the command. """ - def test_returns_true_when_not_aci_environment( - self, monkeypatch: MonkeyPatch - ) -> None: - """Non-ACI environment always passes the validation check.""" - monkeypatch.delenv("ACI_URL", raising=False) + def test_returns_true_when_not_aci_environment(self) -> None: + """Non-ACI environment always passes the validation check. + Relies on the global autouse ``clean_controller_env`` fixture in + ``tests/conftest.py`` ensuring ACI_URL is unset. + """ result = validate_aci_defaults([Path("./data")]) assert result is True diff --git a/tests/unit/core/test_controller.py b/tests/unit/core/test_controller.py index 7369aca6..c0118bcf 100644 --- a/tests/unit/core/test_controller.py +++ b/tests/unit/core/test_controller.py @@ -4,13 +4,11 @@ """Tests for controller type detection utilities.""" import json -import logging import pytest from nac_test.core.constants import ENV_CONTROLLER_CONTEXT from nac_test.core.controller import ( - CONTROLLER_REGISTRY, CredentialSet, IncompleteCredentials, MultipleControllersFound, @@ -18,12 +16,10 @@ _find_credential_sets, _format_multiple_credentials_error, _format_no_credentials_error, - detect_controller_type, format_resolution_error, get_connection_params, get_controller_context, get_controller_url, - get_matched_credential_set, resolve_controller, should_verify_ssl, ) @@ -143,88 +139,60 @@ ] -class TestControllerResolutionContract: - """Contract tests verifying resolve_controller() and detect_controller_type() equivalence. - - These tests ensure the new API (resolve_controller) and deprecated API - (detect_controller_type) return equivalent results. The deprecated function - delegates to resolve_controller(), so these tests catch any drift. - - When Phase 3 removes detect_controller_type(), remove the deprecated assertions - but keep the resolve_controller tests as the primary coverage. - """ +class TestResolveControllerContract: + """Contract tests verifying resolve_controller() behavior.""" @pytest.mark.parametrize( "controller_type,env_vars,expected_auth", CONTROLLER_CREDENTIALS, ids=[f"{c[0]}-{c[2]}" for c in CONTROLLER_CREDENTIALS], ) - def test_success_both_apis_match( + def test_success_resolves_controller_and_auth_method( self, monkeypatch: pytest.MonkeyPatch, controller_type: str, env_vars: dict[str, str], expected_auth: str, ) -> None: - """Both APIs return same controller_type; resolve_controller includes auth_method.""" + """resolve_controller returns ControllerContext with correct type and auth_method.""" for key, value in env_vars.items(): monkeypatch.setenv(key, value) - # New API: returns ControllerContext with type and auth ctx = resolve_controller() assert ctx.controller_type == controller_type assert ctx.auth_method == expected_auth - # Deprecated API: returns just controller_type (delegates to resolve_controller) - deprecated_result = detect_controller_type() - assert deprecated_result == ctx.controller_type, ( - f"Contract violation: detect_controller_type() returned {deprecated_result}, " - f"but resolve_controller().controller_type is {ctx.controller_type}" - ) - - def test_no_credentials_both_apis_raise( + def test_no_credentials_raises_no_credentials_found( self, monkeypatch: pytest.MonkeyPatch ) -> None: - """No credentials: resolve raises NoCredentialsFound, detect raises ValueError.""" - # New API: typed exception + """No credentials: resolve raises NoCredentialsFound.""" with pytest.raises(NoCredentialsFound): resolve_controller() - # Deprecated API: ValueError for backwards compat - with pytest.raises(ValueError) as exc_info: - detect_controller_type() - assert "No controller credentials" in str(exc_info.value) - @pytest.mark.parametrize( "expected_partial,env_vars,scenario", PARTIAL_CREDENTIALS, ids=[f"{c[0]}-{c[2]}" for c in PARTIAL_CREDENTIALS], ) - def test_incomplete_credentials_both_apis_raise( + def test_incomplete_credentials_raises_incomplete_credentials( self, monkeypatch: pytest.MonkeyPatch, expected_partial: str, env_vars: dict[str, str], scenario: str, ) -> None: - """Partial credentials: resolve raises IncompleteCredentials, detect raises ValueError.""" + """Partial credentials: resolve raises IncompleteCredentials with controller list.""" for key, value in env_vars.items(): monkeypatch.setenv(key, value) - # New API: typed exception with controller list with pytest.raises(IncompleteCredentials) as exc_info: resolve_controller() assert expected_partial in exc_info.value.partial_controllers - # Deprecated API: ValueError with same info - with pytest.raises(ValueError) as val_exc: - detect_controller_type() - assert f"{expected_partial}: incomplete credentials" in str(val_exc.value) - - def test_multiple_controllers_both_apis_raise( + def test_multiple_controllers_raises_multiple_controllers_found( self, monkeypatch: pytest.MonkeyPatch ) -> None: - """Multiple complete controllers: both APIs raise appropriately.""" + """Multiple complete controllers: resolve raises MultipleControllersFound.""" # Set ACI credentials monkeypatch.setenv("ACI_URL", "https://apic.local") monkeypatch.setenv("ACI_USERNAME", "admin") @@ -234,24 +202,17 @@ def test_multiple_controllers_both_apis_raise( monkeypatch.setenv("CC_USERNAME", "admin") monkeypatch.setenv("CC_PASSWORD", "pass") - # New API: typed exception with pytest.raises(MultipleControllersFound) as exc_info: resolve_controller() assert "ACI" in exc_info.value.controllers assert "CC" in exc_info.value.controllers - # Deprecated API: ValueError - with pytest.raises(ValueError) as val_exc: - detect_controller_type() - assert "Multiple controller credentials detected" in str(val_exc.value) - class TestGetControllerContext: """Tests for get_controller_context() subprocess accessor. This function is used by PyATS subprocesses to retrieve the resolved - controller context. It reads from NAC_TEST_CONTROLLER_CONTEXT env var - (primary path) or falls back to detect_controller_type() (transitional). + controller context. It deserializes from the NAC_TEST_CONTROLLER_CONTEXT env var. """ def test_reads_from_env_var( @@ -263,22 +224,18 @@ def test_reads_from_env_var( assert result.controller_type == "SDWAN" assert result.auth_method == "session" - def test_fallback_to_detect_when_env_var_absent( - self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture - ) -> None: - """Transitional fallback: invokes detect_controller_type() with info log.""" - # Set controller credentials (fallback path will detect) + def test_raises_when_env_var_absent(self, monkeypatch: pytest.MonkeyPatch) -> None: + """When NAC_TEST_CONTROLLER_CONTEXT is absent, raises ValueError.""" + # Set controller credentials in env (which no longer fall back) monkeypatch.setenv("ACI_URL", "https://apic.local") monkeypatch.setenv("ACI_USERNAME", "admin") monkeypatch.setenv("ACI_PASSWORD", "pass") - # NAC_TEST_CONTROLLER_CONTEXT deliberately not set + monkeypatch.delenv(ENV_CONTROLLER_CONTEXT, raising=False) - with caplog.at_level(logging.INFO, logger="nac_test.core.controller"): - ctx = get_controller_context() + with pytest.raises(ValueError) as exc_info: + get_controller_context() - assert ctx.controller_type == "ACI" - assert ctx.auth_method == "session" - assert "falling back to detect_controller_type" in caplog.text + assert ENV_CONTROLLER_CONTEXT in str(exc_info.value) class TestControllerContextSerialization: @@ -439,10 +396,8 @@ def test_case_sensitivity(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("aci_username", "admin") monkeypatch.setenv("aci_password", "password") - with pytest.raises(ValueError) as exc_info: - detect_controller_type() - - assert "No controller credentials found" in str(exc_info.value) + with pytest.raises(NoCredentialsFound): + resolve_controller() def test_special_characters_in_credentials( self, monkeypatch: pytest.MonkeyPatch @@ -453,8 +408,9 @@ def test_special_characters_in_credentials( monkeypatch.setenv("CC_USERNAME", "user@domain.com") monkeypatch.setenv("CC_PASSWORD", "p@$$w0rd!#$%^&*()") - result = detect_controller_type() - assert result == "CC" + ctx = resolve_controller() + assert ctx.controller_type == "CC" + assert ctx.auth_method == "session" def test_legacy_controller_type_ignored( self, monkeypatch: pytest.MonkeyPatch @@ -468,9 +424,9 @@ def test_legacy_controller_type_ignored( monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") - result = detect_controller_type() + ctx = resolve_controller() assert ( - result == "SDWAN" + ctx.controller_type == "SDWAN" ) # Should use credential-based detection, not CONTROLLER_TYPE def test_mixed_complete_and_partial_credentials( @@ -486,8 +442,8 @@ def test_mixed_complete_and_partial_credentials( monkeypatch.setenv("ISE_URL", "https://ise.example.com") monkeypatch.setenv("ISE_USERNAME", "ise_admin") - result = detect_controller_type() - assert result == "FMC" # Should detect the complete set + ctx = resolve_controller() + assert ctx.controller_type == "FMC" # Should detect the complete set def test_whitespace_trimming_in_values( self, monkeypatch: pytest.MonkeyPatch @@ -498,22 +454,18 @@ def test_whitespace_trimming_in_values( monkeypatch.setenv("MERAKI_USERNAME", " admin ") monkeypatch.setenv("MERAKI_PASSWORD", " password ") - result = detect_controller_type() - assert result == "MERAKI" + ctx = resolve_controller() + assert ctx.controller_type == "MERAKI" - def test_truly_empty_environment(self, monkeypatch: pytest.MonkeyPatch) -> None: - """Test with a completely empty environment.""" - # Clear all controller-related environment variables - for config in CONTROLLER_REGISTRY.values(): - for cred_set in config.credential_sets: - for var in cred_set.env_vars: - monkeypatch.delenv(var, raising=False) + def test_truly_empty_environment(self) -> None: + """When no controller env vars are set, raises NoCredentialsFound. - with pytest.raises(ValueError) as exc_info: - detect_controller_type() - - error_msg = str(exc_info.value) - assert "No controller credentials found" in error_msg + Relies on the global autouse ``clean_controller_env`` fixture in + ``tests/conftest.py`` ensuring all controller environment variables + are unset. + """ + with pytest.raises(NoCredentialsFound): + resolve_controller() def test_three_way_multiple_controllers( self, monkeypatch: pytest.MonkeyPatch @@ -532,10 +484,11 @@ def test_three_way_multiple_controllers( monkeypatch.setenv("ISE_USERNAME", "ise_user") monkeypatch.setenv("ISE_PASSWORD", "ise_pass") - with pytest.raises(ValueError) as exc_info: - detect_controller_type() + with pytest.raises(MultipleControllersFound) as exc_info: + resolve_controller() - error_msg = str(exc_info.value) + assert set(exc_info.value.controllers) == {"ACI", "CC", "ISE"} + error_msg = format_resolution_error(exc_info.value) assert "Multiple controller credentials detected: ACI, CC, ISE" in error_msg assert "To use ACI only:" in error_msg assert "To use CC only:" in error_msg @@ -548,8 +501,8 @@ def test_unicode_in_credentials(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("ACI_USERNAME", "用户名") # Chinese characters monkeypatch.setenv("ACI_PASSWORD", "пароль") # Cyrillic characters - result = detect_controller_type() - assert result == "ACI" + ctx = resolve_controller() + assert ctx.controller_type == "ACI" def test_url_with_path_and_query(self, monkeypatch: pytest.MonkeyPatch) -> None: """Test URL values with paths and query parameters.""" @@ -559,8 +512,8 @@ def test_url_with_path_and_query(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") - result = detect_controller_type() - assert result == "SDWAN" + ctx = resolve_controller() + assert ctx.controller_type == "SDWAN" def test_iosxe_partial_and_sdwan_partial_are_both_reported( self, monkeypatch: pytest.MonkeyPatch @@ -579,10 +532,12 @@ def test_iosxe_partial_and_sdwan_partial_are_both_reported( monkeypatch.setenv("SDWAN_URL", "https://vmanage.example.com") # No SDWAN credentials beyond URL - with pytest.raises(ValueError) as exc_info: - detect_controller_type() + with pytest.raises(IncompleteCredentials) as exc_info: + resolve_controller() - error_msg = str(exc_info.value) + assert "IOSXE" in exc_info.value.partial_controllers + assert "SDWAN" in exc_info.value.partial_controllers + error_msg = format_resolution_error(exc_info.value) assert "Incomplete controller credentials detected" in error_msg assert "IOSXE: incomplete credentials" in error_msg assert "SDWAN: incomplete credentials" in error_msg @@ -594,10 +549,11 @@ def test_empty_string_handling(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("ACI_USERNAME", "admin") monkeypatch.setenv("ACI_PASSWORD", "") # Empty string - with pytest.raises(ValueError) as exc_info: - detect_controller_type() + with pytest.raises(IncompleteCredentials) as exc_info: + resolve_controller() - error_msg = str(exc_info.value) + assert "ACI" in exc_info.value.partial_controllers + error_msg = format_resolution_error(exc_info.value) assert "Incomplete controller credentials detected" in error_msg assert "ACI: incomplete credentials" in error_msg @@ -608,10 +564,11 @@ def test_whitespace_handling(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", " ") # Only whitespace - with pytest.raises(ValueError) as exc_info: - detect_controller_type() + with pytest.raises(IncompleteCredentials) as exc_info: + resolve_controller() - error_msg = str(exc_info.value) + assert "SDWAN" in exc_info.value.partial_controllers + error_msg = format_resolution_error(exc_info.value) assert "Incomplete controller credentials detected" in error_msg assert "SDWAN: incomplete credentials" in error_msg @@ -628,8 +585,8 @@ def test_d2d_scenario_with_dummy_credentials( monkeypatch.setenv("IOSXE_USERNAME", "device_user") monkeypatch.setenv("IOSXE_PASSWORD", "device_pass") - result = detect_controller_type() - assert result == "ACI" # Controller type still detected + ctx = resolve_controller() + assert ctx.controller_type == "ACI" # Controller type still detected class TestIOSXEAlternativeURLEnvVar: @@ -645,8 +602,8 @@ def test_detect_iosxe_with_host(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("IOSXE_USERNAME", "admin") monkeypatch.setenv("IOSXE_PASSWORD", "password") - result = detect_controller_type() - assert result == "IOSXE" + ctx = resolve_controller() + assert ctx.controller_type == "IOSXE" def test_iosxe_url_takes_precedence_over_host( self, monkeypatch: pytest.MonkeyPatch @@ -657,8 +614,8 @@ def test_iosxe_url_takes_precedence_over_host( monkeypatch.setenv("IOSXE_USERNAME", "admin") monkeypatch.setenv("IOSXE_PASSWORD", "password") - result = detect_controller_type() - assert result == "IOSXE" + ctx = resolve_controller() + assert ctx.controller_type == "IOSXE" # Verify URL takes precedence in get_controller_url url = get_controller_url("IOSXE") @@ -772,14 +729,9 @@ def test_detect_sdwan_with_api_token(self, monkeypatch: pytest.MonkeyPatch) -> N monkeypatch.setenv("SDWAN_URL", "https://vmanage.example.com") monkeypatch.setenv("SDWAN_API_TOKEN", "eyJhbGciOiJSUzI1NiJ9.test.sig") - result = detect_controller_type() - assert result == "SDWAN" - - # Token set should be matched with auth_method="token" - cred = get_matched_credential_set("SDWAN") - assert cred is not None - assert cred.auth_method == "token" - assert cred.label == "API Token (20.18+)" + ctx = resolve_controller() + assert ctx.controller_type == "SDWAN" + assert ctx.auth_method == "token" def test_detect_sdwan_with_username_password( self, monkeypatch: pytest.MonkeyPatch @@ -789,14 +741,9 @@ def test_detect_sdwan_with_username_password( monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") - result = detect_controller_type() - assert result == "SDWAN" - - # Password set should be matched with auth_method="session" - cred = get_matched_credential_set("SDWAN") - assert cred is not None - assert cred.auth_method == "session" - assert cred.label == "Username/Password" + ctx = resolve_controller() + assert ctx.controller_type == "SDWAN" + assert ctx.auth_method == "session" def test_api_token_takes_priority(self, monkeypatch: pytest.MonkeyPatch) -> None: """When both credential sets are satisfied, token set wins (listed first).""" @@ -805,14 +752,9 @@ def test_api_token_takes_priority(self, monkeypatch: pytest.MonkeyPatch) -> None monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") - # Should still detect exactly one SDWAN (not duplicate) - result = detect_controller_type() - assert result == "SDWAN" - - # Token set wins because it's listed first - cred = get_matched_credential_set("SDWAN") - assert cred is not None - assert cred.auth_method == "token" + ctx = resolve_controller() + assert ctx.controller_type == "SDWAN" + assert ctx.auth_method == "token" def test_partial_token_set_falls_back_to_password( self, monkeypatch: pytest.MonkeyPatch @@ -823,13 +765,9 @@ def test_partial_token_set_falls_back_to_password( monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") - result = detect_controller_type() - assert result == "SDWAN" - - # Password set matched because token set was incomplete - cred = get_matched_credential_set("SDWAN") - assert cred is not None - assert cred.auth_method == "session" + ctx = resolve_controller() + assert ctx.controller_type == "SDWAN" + assert ctx.auth_method == "session" def test_empty_api_token_falls_back_to_password( self, monkeypatch: pytest.MonkeyPatch @@ -840,31 +778,24 @@ def test_empty_api_token_falls_back_to_password( monkeypatch.setenv("SDWAN_USERNAME", "admin") monkeypatch.setenv("SDWAN_PASSWORD", "password") - result = detect_controller_type() - assert result == "SDWAN" - - # Should fall back to session auth - cred = get_matched_credential_set("SDWAN") - assert cred is not None - assert cred.auth_method == "session" + ctx = resolve_controller() + assert ctx.controller_type == "SDWAN" + assert ctx.auth_method == "session" def test_url_only_is_partial(self, monkeypatch: pytest.MonkeyPatch) -> None: """SDWAN_URL alone (no token, no username/password) is partial.""" monkeypatch.setenv("SDWAN_URL", "https://vmanage.example.com") - with pytest.raises(ValueError) as exc_info: - detect_controller_type() + with pytest.raises(IncompleteCredentials) as exc_info: + resolve_controller() - error_msg = str(exc_info.value) + assert "SDWAN" in exc_info.value.partial_controllers + error_msg = format_resolution_error(exc_info.value) assert "Incomplete controller credentials detected" in error_msg assert "SDWAN: incomplete credentials" in error_msg assert "API Token (20.18+)" in error_msg assert "Username/Password" in error_msg - def test_get_matched_credential_set_before_detection(self) -> None: - """get_matched_credential_set returns None before detect_controller_type runs.""" - assert get_matched_credential_set("SDWAN") is None - def test_credential_set_auth_method_default(self) -> None: """CredentialSet.auth_method defaults to 'session'.""" cs = CredentialSet( @@ -874,17 +805,14 @@ def test_credential_set_auth_method_default(self) -> None: assert cs.auth_method == "session" def test_aci_matched_credential_set(self, monkeypatch: pytest.MonkeyPatch) -> None: - """ACI detection stores matched credential set with session auth.""" + """ACI detection resolves controller with session auth.""" monkeypatch.setenv("ACI_URL", "https://apic.example.com") monkeypatch.setenv("ACI_USERNAME", "admin") monkeypatch.setenv("ACI_PASSWORD", "password") - detect_controller_type() - - cred = get_matched_credential_set("ACI") - assert cred is not None - assert cred.auth_method == "session" - assert cred.label == "Username/Password" + ctx = resolve_controller() + assert ctx.controller_type == "ACI" + assert ctx.auth_method == "session" class TestGetControllerUrlSDWAN: @@ -999,8 +927,6 @@ def test_missing_env_vars_raises_value_error( ) -> None: """Unset env vars raise ValueError listing the missing var names.""" monkeypatch.setenv("ACI_URL", "https://apic.example.com") - monkeypatch.delenv("ACI_USERNAME", raising=False) - monkeypatch.delenv("ACI_PASSWORD", raising=False) with pytest.raises(ValueError) as exc_info: get_connection_params("ACI", AuthMethod.SESSION) @@ -1061,7 +987,6 @@ def test_iosxe_host_variant_resolves_when_url_unset( Both IOSXE_URL and IOSXE_HOST share auth_method="session", so the first fully-satisfied candidate must win - not just the first one in order. """ - monkeypatch.delenv("IOSXE_URL", raising=False) monkeypatch.setenv("IOSXE_HOST", "192.168.1.1") monkeypatch.setenv("IOSXE_USERNAME", "admin") monkeypatch.setenv("IOSXE_PASSWORD", "password") @@ -1075,14 +1000,9 @@ def test_iosxe_host_variant_resolves_when_url_unset( } def test_iosxe_reports_url_variant_missing_vars_when_nothing_configured( - self, monkeypatch: pytest.MonkeyPatch + self, ) -> None: """With neither variant configured, the first (URL) set's vars are reported.""" - monkeypatch.delenv("IOSXE_URL", raising=False) - monkeypatch.delenv("IOSXE_HOST", raising=False) - monkeypatch.delenv("IOSXE_USERNAME", raising=False) - monkeypatch.delenv("IOSXE_PASSWORD", raising=False) - with pytest.raises(ValueError) as exc_info: get_connection_params("IOSXE", AuthMethod.SESSION) @@ -1096,10 +1016,7 @@ def test_iosxe_reports_host_variant_missing_vars_when_partially_configured( not IOSXE_URL - the caller never touched the URL variant, so the error must point at the variant they actually started configuring. """ - monkeypatch.delenv("IOSXE_URL", raising=False) monkeypatch.setenv("IOSXE_HOST", "192.168.1.1") - monkeypatch.delenv("IOSXE_USERNAME", raising=False) - monkeypatch.delenv("IOSXE_PASSWORD", raising=False) with pytest.raises(ValueError) as exc_info: get_connection_params("IOSXE", AuthMethod.SESSION) @@ -1113,10 +1030,8 @@ def test_iosxe_reports_host_variant_missing_vars_when_partially_configured( class TestShouldVerifySsl: """Tests for should_verify_ssl().""" - def test_defaults_false_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + def test_defaults_false_when_unset(self) -> None: """Unset env var defaults to False (skip verify), matching prior adapter behavior.""" - monkeypatch.delenv("ACI_INSECURE", raising=False) - assert should_verify_ssl("ACI") is False def test_defaults_false_when_empty(self, monkeypatch: pytest.MonkeyPatch) -> None: @@ -1143,12 +1058,8 @@ def test_insecure_falsy_means_verify( assert should_verify_ssl("SDWAN") is True - def test_custom_default_used_when_unset( - self, monkeypatch: pytest.MonkeyPatch - ) -> None: + def test_custom_default_used_when_unset(self) -> None: """The `default` param controls the unset fallback.""" - monkeypatch.delenv("ISE_INSECURE", raising=False) - assert should_verify_ssl("ISE", default=True) is True def test_unknown_controller_type_raises_key_error(self) -> None: diff --git a/tests/unit/core/test_controller_auth.py b/tests/unit/core/test_controller_auth.py index f72bff70..b7d8dea9 100644 --- a/tests/unit/core/test_controller_auth.py +++ b/tests/unit/core/test_controller_auth.py @@ -6,11 +6,13 @@ ensuring authentication failures are identified and classified appropriately. """ +import os from typing import get_args from _pytest.monkeypatch import MonkeyPatch from pytest_mock import MockerFixture +from nac_test.core.constants import ENV_CONTROLLER_CONTEXT from nac_test.core.controller_auth import ( CONTROLLER_REGISTRY, AuthOutcome, @@ -116,6 +118,7 @@ def test_returns_success_when_auth_succeeds( assert result.reason == AuthOutcome.SUCCESS assert result.controller_type == "ACI" assert result.controller_url == "https://apic.example.com" + assert os.environ.get(ENV_CONTROLLER_CONTEXT) == aci_context.to_json() mock_auth.assert_called_once() def test_returns_failure_for_bad_credentials( diff --git a/tests/unit/pyats_core/common/test_base_test_iosxe_credentials.py b/tests/unit/pyats_core/common/test_base_test_iosxe_credentials.py index 01fcee77..d0a007c1 100644 --- a/tests/unit/pyats_core/common/test_base_test_iosxe_credentials.py +++ b/tests/unit/pyats_core/common/test_base_test_iosxe_credentials.py @@ -16,6 +16,9 @@ import pytest +from nac_test.core.controller import IncompleteCredentials, resolve_controller +from tests.conftest import resolve_and_inject_context + @pytest.fixture() def temp_data_model_file( @@ -44,10 +47,10 @@ def test_iosxe_setup_fails_without_username_password( iosxe_controller_env: None, monkeypatch: pytest.MonkeyPatch, ) -> None: - """setup() should fail for IOSXE without USERNAME/PASSWORD. + """resolve_controller() should fail for IOSXE without USERNAME/PASSWORD. - IOSXE now requires IOSXE_USERNAME and IOSXE_PASSWORD in addition to - IOSXE_URL (or IOSXE_HOST). Detection should report incomplete credentials. + IOSXE requires IOSXE_USERNAME and IOSXE_PASSWORD in addition to + IOSXE_URL (or IOSXE_HOST). Resolution reports incomplete credentials. """ # Remove USERNAME and PASSWORD to simulate incomplete IOSXE environment monkeypatch.delenv("IOSXE_USERNAME", raising=False) @@ -58,31 +61,26 @@ def test_iosxe_setup_fails_without_username_password( assert "IOSXE_USERNAME" not in os.environ assert "IOSXE_PASSWORD" not in os.environ - instance = nac_test_base_class.__new__(nac_test_base_class) - - # setup() should fail with incomplete credentials - with pytest.raises(ValueError) as exc_info: - instance.setup() + with pytest.raises(IncompleteCredentials) as exc_info: + resolve_controller() - error_msg = str(exc_info.value) - assert "Incomplete controller credentials detected" in error_msg - assert "IOSXE" in error_msg + assert "IOSXE" in exc_info.value.partial_controllers def test_iosxe_setup_works_with_username_password( self, nac_test_base_class: Any, temp_data_model_file: Path, iosxe_controller_env: None, + monkeypatch: pytest.MonkeyPatch, ) -> None: - """setup() should also work if IOSXE USERNAME/PASSWORD are provided. - - While not required, if someone sets them, we should accept them. - """ + """setup() works when IOSXE credentials and context are provided.""" # Verify all credentials are set assert "IOSXE_URL" in os.environ assert "IOSXE_USERNAME" in os.environ assert "IOSXE_PASSWORD" in os.environ + resolve_and_inject_context(monkeypatch) + instance = nac_test_base_class.__new__(nac_test_base_class) instance.setup() @@ -96,6 +94,7 @@ def test_aci_setup_requires_username_password( nac_test_base_class: Any, temp_data_model_file: Path, aci_controller_env: None, + monkeypatch: pytest.MonkeyPatch, ) -> None: """setup() should succeed for ACI with all required credentials. @@ -107,6 +106,8 @@ def test_aci_setup_requires_username_password( assert "ACI_USERNAME" in os.environ assert "ACI_PASSWORD" in os.environ + resolve_and_inject_context(monkeypatch) + instance = nac_test_base_class.__new__(nac_test_base_class) instance.setup() @@ -122,18 +123,14 @@ def test_aci_setup_fails_without_username( aci_controller_env: None, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Controller detection should fail for ACI without USERNAME. + """Controller resolution should fail for ACI without USERNAME. - ACI requires all three credentials - detect_controller_type() should - raise ValueError for incomplete credentials before setup() reads them. + ACI requires all three credentials - resolve_controller() should + raise IncompleteCredentials. """ monkeypatch.delenv("ACI_USERNAME", raising=False) - instance = nac_test_base_class.__new__(nac_test_base_class) - - # setup() should fail during controller detection, not when reading env vars - with pytest.raises(ValueError) as exc_info: - instance.setup() + with pytest.raises(IncompleteCredentials) as exc_info: + resolve_controller() - assert "Incomplete controller credentials" in str(exc_info.value) - assert "ACI: incomplete credentials" in str(exc_info.value) + assert "ACI" in exc_info.value.partial_controllers diff --git a/tests/unit/pyats_core/common/test_ssh_base_test.py b/tests/unit/pyats_core/common/test_ssh_base_test.py index 50bfe618..92e48d25 100644 --- a/tests/unit/pyats_core/common/test_ssh_base_test.py +++ b/tests/unit/pyats_core/common/test_ssh_base_test.py @@ -16,6 +16,7 @@ from nac_test.pyats_core.common.ssh_base_test import SSHTestBase from nac_test.pyats_core.constants import DEVICE_EXECUTE_TIMEOUT from nac_test.pyats_core.ssh.command_cache import CommandCache +from tests.conftest import resolve_and_inject_context @pytest.fixture() @@ -48,6 +49,7 @@ def test_validation_called_for_valid_device( monkeypatch: pytest.MonkeyPatch, ) -> None: """Validation passes for a fully-populated device info dict.""" + resolve_and_inject_context(monkeypatch) valid_device = { "hostname": "test-router", "host": "192.168.1.1", @@ -78,6 +80,7 @@ def test_validation_fails_for_missing_fields( monkeypatch: pytest.MonkeyPatch, ) -> None: """Validation fails with a clear message when required fields are absent.""" + resolve_and_inject_context(monkeypatch) invalid_device = { "hostname": "test-router", "host": "192.168.1.1", @@ -103,6 +106,7 @@ def test_validation_not_called_for_json_parse_error( monkeypatch: pytest.MonkeyPatch, ) -> None: """Validation is skipped when JSON parsing fails.""" + resolve_and_inject_context(monkeypatch) monkeypatch.setenv("DEVICE_INFO", "not valid json") instance = self._make_instance()