feat(introspection): add config introspection for tracing value origins#41
feat(introspection): add config introspection for tracing value origins#41mischadiehm merged 9 commits intomainfrom
Conversation
Implement "Where did X come from?" feature inspired by Dynaconf pattern. Enables users to trace configuration value sources via `nw info --trace`. - Add introspection.py with LoaderType, FieldHistory, ConfigHistory types - Add _history tracking to DeviceConfig, DeviceGroup, GeneralConfig models - Add CredentialSource and resolve_credentials_with_source() to credentials.py - Add SSH config provenance tracking in sync_ssh.py - Add --trace flag to info command for source column display - Add unit tests for introspection module - Add integration tests for --trace output Closes #37, #38, #39
…nfig history - Extract ConfigHistoryMixin for shared history tracking methods - Make _resolve_username/password delegate to _with_source() variants - Remove value field from CredentialSource (security improvement) - Remove unused CredentialResolutionTrace class - Add sensitive field masking to ConfigHistory.to_dict() - Simplify _get_credential_source() to use resolver - Fix path string replacement with proper Path operations Net reduction: -159 lines, eliminates duplicate resolution logic
- Create docs/reference/introspection.md documenting: - CLI usage with --trace flag - Source types (config_file, env_var, dotenv, group, etc.) - Python API (FieldHistory, ConfigHistory, ConfigHistoryMixin) - Remove unused to_dict() method and sensitive field masking logic - Remove 4 tests for dead code (21 tests remain, all passing) - Add introspection docs to mkdocs.yml navigation
- Remove unused `merged` field from FieldHistory dataclass - Remove unused `clear()` method from ConfigHistory - Add docstring comments marking reserved LoaderTypes (DOTENV, CLI, INTERACTIVE) - Update docs to distinguish implemented vs reserved source types - Remove 1 test for deleted clear() method (20 tests remain)
Update CredentialResolver to properly distinguish CLI flag overrides (--user/--password) from interactive prompts by using LoaderType.CLI instead of LoaderType.INTERACTIVE. - CLI flag overrides now use LoaderType.CLI with no identifier - Add test_format_source_cli for CLI source formatting - Add TestCredentialSourceTracking with comprehensive tests
- Mark CLI as implemented (not reserved) in LoaderType docstring - Add meaningful assertions to trace tests for Source column and paths - Improve test robustness for terminal width truncation
| from dataclasses import dataclass | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from network_toolkit.introspection import LoaderType |
Check failure
Code scanning / CodeQL
Module-level cyclic import Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, we should break the cyclic import at the type level while preserving type information. Since the imports of DeviceConfig and NetworkConfig are used only for type checking (inside if TYPE_CHECKING:) and not at runtime, the best approach is to avoid importing them altogether and instead use forward-referenced (string) type annotations. This removes the dependency of network_toolkit.credentials on network_toolkit.config, breaking the cycle without changing runtime behavior.
Concretely, in src/network_toolkit/credentials.py:
- Remove the
if TYPE_CHECKING:block that importsDeviceConfigandNetworkConfig. - Update any annotations that directly reference
NetworkConfigorDeviceConfigto use string-based forward references instead (e.g.,config: "NetworkConfig"), which type checkers understand but do not require importing the types. - No other functionality needs to change; we only adjust type hints and the TYPE_CHECKING import block.
The key region to modify is:
- Lines 15–16: the
if TYPE_CHECKING:block. - The
__init__method ofCredentialResolver(and any other methods/attributes inside this file, if present in elided sections, that annotate withNetworkConfigorDeviceConfig) so that they refer to"NetworkConfig"/"DeviceConfig"as strings instead of names requiring imports.
| @@ -12,10 +12,10 @@ | ||
|
|
||
| from network_toolkit.introspection import LoaderType | ||
|
|
||
| if TYPE_CHECKING: | ||
| from network_toolkit.config import DeviceConfig, NetworkConfig | ||
|
|
||
|
|
||
|
|
||
|
|
||
| # module logger | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
| @@ -61,7 +59,7 @@ | ||
| 5. Default environment variables | ||
| """ | ||
|
|
||
| def __init__(self, config: NetworkConfig) -> None: | ||
| def __init__(self, config: "NetworkConfig") -> None: | ||
| """Initialize with network configuration.""" | ||
| self.config = config | ||
|
|
The `nw info <sequence_name>` command was failing with AttributeError because get_sequence_record() was called but never implemented. - Add get_sequence_record(sequence_name, vendor) method - Add integration tests for sequence info display
- Change show_provenance default to True in DeviceInfoTableProvider - Add verbose_provenance parameter for full paths vs compact display - Repurpose --trace for verbose provenance (full paths + line numbers) - Update format_source() with verbose keyword-only parameter - Update tests to expect Source column always present
- test_format_source_config_file_with_line: use verbose=True for line numbers - test_info_with_trace_shows_source_column: check for env:/default (robust) - test_info_help_shows_trace: strip ANSI codes before assertion
Address findings from CodeQL analysis during PR #41 review: - Skip incomplete integration test with @pytest.mark.skip decorator - The test was a placeholder that ended with bare `pass` statement - Reference issue #42 in skip reason for future completion Other CodeQL findings analyzed as false positives: - Clear-text logging: Code uses safe_keys allowlist filtering - Cyclic imports: Intentional lazy import pattern at runtime - Empty except blocks: Intentional error recovery patterns Closes #42
Address findings from CodeQL analysis during PR #41 review: - Skip incomplete integration test with @pytest.mark.skip decorator - The test was a placeholder that ended with bare `pass` statement - Reference issue #42 in skip reason for future completion Other CodeQL findings analyzed as false positives: - Clear-text logging: Code uses safe_keys allowlist filtering - Cyclic imports: Intentional lazy import pattern at runtime - Empty except blocks: Intentional error recovery patterns Closes #42 * fix(introspection): add missing vendor_specific field to VendorSequenceInfoTableProvider The VendorSequenceInfoTableProvider class was using self.vendor_specific in get_table_definition() but the field was not defined on the class. This caused AttributeError when displaying vendor sequences without the --vendor flag. Added vendor_specific: bool = False as a class field with default False.
Summary
nw info <device> --traceflag to show source provenance for each configuration fieldLoaderType,FieldHistory, andConfigHistoryclassesConfigHistoryMixinfor consistent history tracking across config classesCredentialSourcedataclass in credential resolutiondocs/reference/introspection.mdnw info <device>output (always-on introspection)--tracefor verbose provenance (full file paths + line numbers) instead of toggling visibilityChanges
New Module:
src/network_toolkit/introspection.pyLoaderTypeenum: CONFIG_FILE, ENV_VAR, GROUP, SSH_CONFIG, PYDANTIC_DEFAULT, CLIFieldHistoryfrozen dataclass for immutable value recordsConfigHistorycontainer for tracking field value historyformat_source()method withverboseparameter for compact vs full-path displayConfig Integration:
src/network_toolkit/config.pyConfigHistoryMixinprovidingrecord_field(),get_field_history(),get_field_source()_populate_device_field_history()and_populate_group_field_history()functionsCredential Tracking:
src/network_toolkit/credentials.pyCredentialSourcedataclass (intentionally does not store credential values)resolve_credentials_with_source()method returning source alongside credentialsLoaderType.CLICLI:
src/network_toolkit/commands/info.py--trace/-tflag enables verbose provenance with full file pathsTable Providers:
src/network_toolkit/common/table_providers.pyDeviceInfoTableProvider.show_provenancedefault changed toTrueverbose_provenanceparameter for compact vs verbose displayBehavior
nw info router1- Shows Source column with compact sources (e.g.,devices.yml,env: NW_USER)nw info router1 --trace- Shows Source column with full paths (e.g.,/Users/md/.config/networka/devices/devices.yml)Test Plan
uv run pytest tests/test_introspection.py -v- 23 tests passuv run pytest tests/test_info_trace.py -v- 14 tests passuv run pytest tests/ -q- All tests passuv run pre-commit run --all-files- all checks passnw info <device>andnw info <device> --tracewith real configChecklist
Deferred Work
None - all planned work is complete.
Known Limitations
Value-equality based source detection in
_populate_device_field_history()may incorrectly attribute a user-set value matching the Pydantic default (e.g.,port: 22) as "default" rather than "config file". This would require parsing-level changes to fix properly.