diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c20cc1..64881df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,9 +35,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 their driver modules (e.g. `remove_ipv4_acl_remarks` in `hier_config.platforms.cisco_ios.driver`), so a built-in callback can be removed by identity with `rules.post_load_callbacks.remove(...)` (#286). +- The driver registry is keyed internally on canonical uppercase platform + names; `Platform` members are converted via their names at the boundary, so + a member and its name are fully interchangeable in `register_driver`, + `unregister_driver`, and `get_hconfig_driver`. `get_registered_platforms()` + returns `Platform` members for enum-known names and uppercase strings for + custom names (#284). ### Fixed +- Registering a driver under a `Platform` member's *value* string (e.g. `"3"`, + the value of `Platform.CISCO_IOS`) no longer silently overwrites that + platform's built-in registry entry, and value strings no longer resolve in + platform lookups — platforms are identified by name (#284). - `future()` negation edge cases (#269): a negation whose positive form exists in the running config now removes it without surviving as a literal `no ...` child (evaluated before the idempotency rules, which can match the negation diff --git a/docs/admin/custom-drivers.md b/docs/admin/custom-drivers.md index bb0e1db..924ae16 100644 --- a/docs/admin/custom-drivers.md +++ b/docs/admin/custom-drivers.md @@ -94,6 +94,8 @@ config = HConfig.from_text("MY_NOS", config_text) config = HConfig.from_text("my_nos", config_text) # same driver ``` +Names are canonicalized to uppercase, and a `Platform` member is interchangeable with its name — `register_driver("cisco_ios", ...)` and `register_driver(Platform.CISCO_IOS, ...)` address the same entry. + The registry is not synchronized — register drivers at application startup, before configs are parsed concurrently. ### Overriding a built-in driver @@ -131,6 +133,8 @@ print(get_registered_platforms()) # (Platform.ARISTA_EOS, Platform.CISCO_IOS, ..., 'MY_NOS') ``` +Names known to the `Platform` enum are returned as members; custom names are returned as canonical uppercase strings. + ## Using an unregistered driver instance Registration is optional. Every constructor also accepts a driver *instance* directly, which is convenient for one-off customizations: diff --git a/docs/dev/architecture.md b/docs/dev/architecture.md index a0cadba..5474682 100644 --- a/docs/dev/architecture.md +++ b/docs/dev/architecture.md @@ -113,11 +113,11 @@ See the [Driver Rule Reference](rule-reference.md) for every model's fields. ### Registry (`hier_config/registry.py`) -Built-in drivers are registered at import time in a module-level registry mapping `Platform | str` → driver class: +Built-in drivers are registered at import time in a module-level registry keyed on canonical uppercase platform names — `Platform` members are converted via their `.name`, string names are uppercased, so a member and its name address the same entry (#284): - `register_driver(platform, driver_class)` — add a custom platform (string names, case-insensitive) or override a built-in. - `unregister_driver(platform)` — remove a custom platform or restore an overridden built-in. -- `get_registered_platforms()` — list everything registered. +- `get_registered_platforms()` — list everything registered: `Platform` members for enum-known names, uppercase strings for custom names. - `get_hconfig_driver(platform)` — instantiate the registered driver. - `resolve_driver(platform_or_driver)` — accept a `Platform`, string, or driver instance (used by every constructor). diff --git a/hier_config/registry.py b/hier_config/registry.py index a1467aa..a12ff61 100644 --- a/hier_config/registry.py +++ b/hier_config/registry.py @@ -4,6 +4,10 @@ custom platforms (by string name), override built-in drivers, and restore built-in defaults by unregistering the override. +Entries are keyed on canonical uppercase platform names (#284): `Platform` +members are converted via their names at the boundary, and string names are +uppercased, so a member and its name address the same entry. + The registry is not synchronized; register drivers at application startup, before configs are parsed concurrently. """ @@ -25,33 +29,31 @@ from hier_config.platforms.nokia_srl.driver import HConfigDriverNokiaSRL from hier_config.platforms.vyos.driver import HConfigDriverVYOS -_BUILTIN_DRIVERS: dict[Platform | str, type[HConfigDriverBase]] = { - Platform.ARISTA_EOS: HConfigDriverAristaEOS, - Platform.ARUBA_AOSCX: HConfigDriverArubaAOSCX, - Platform.CISCO_IOS: HConfigDriverCiscoIOS, - Platform.CISCO_NXOS: HConfigDriverCiscoNXOS, - Platform.CISCO_XR: HConfigDriverCiscoIOSXR, - Platform.FORTINET_FORTIOS: HConfigDriverFortinetFortiOS, - Platform.GENERIC: HConfigDriverGeneric, - Platform.HP_PROCURVE: HConfigDriverHPProcurve, - Platform.HP_COMWARE5: HConfigDriverHPComware5, - Platform.HUAWEI_VRP: HConfigDriverHuaweiVrp, - Platform.JUNIPER_JUNOS: HConfigDriverJuniperJUNOS, - Platform.NOKIA_SRL: HConfigDriverNokiaSRL, - Platform.VYOS: HConfigDriverVYOS, +_BUILTIN_DRIVERS: dict[str, type[HConfigDriverBase]] = { + Platform.ARISTA_EOS.name: HConfigDriverAristaEOS, + Platform.ARUBA_AOSCX.name: HConfigDriverArubaAOSCX, + Platform.CISCO_IOS.name: HConfigDriverCiscoIOS, + Platform.CISCO_NXOS.name: HConfigDriverCiscoNXOS, + Platform.CISCO_XR.name: HConfigDriverCiscoIOSXR, + Platform.FORTINET_FORTIOS.name: HConfigDriverFortinetFortiOS, + Platform.GENERIC.name: HConfigDriverGeneric, + Platform.HP_PROCURVE.name: HConfigDriverHPProcurve, + Platform.HP_COMWARE5.name: HConfigDriverHPComware5, + Platform.HUAWEI_VRP.name: HConfigDriverHuaweiVrp, + Platform.JUNIPER_JUNOS.name: HConfigDriverJuniperJUNOS, + Platform.NOKIA_SRL.name: HConfigDriverNokiaSRL, + Platform.VYOS.name: HConfigDriverVYOS, } -_registry: dict[Platform | str, type[HConfigDriverBase]] = dict(_BUILTIN_DRIVERS) +_registry: dict[str, type[HConfigDriverBase]] = dict(_BUILTIN_DRIVERS) -def _normalize(platform: Platform | str) -> Platform | str: +def _normalize(platform: Platform | str) -> str: + # Platform must be checked first: it subclasses str, and its str content + # is the enum value, not the platform name. if isinstance(platform, Platform): - return platform - name = platform.upper() - try: - return Platform[name] - except KeyError: - return name + return platform.name + return platform.upper() def register_driver( @@ -61,30 +63,39 @@ def register_driver( """Register a driver for a platform. Passing a string registers a custom platform usable anywhere a `Platform` - is accepted (names are case-insensitive). Passing an existing `Platform` - member overrides the built-in driver for that platform. + is accepted; names are canonicalized to uppercase, so registration and + lookup are case-insensitive. Passing an existing `Platform` member (or its + name — the two are interchangeable) overrides the built-in driver for + that platform. """ _registry[_normalize(platform)] = driver_class def unregister_driver(platform: Platform | str) -> None: """Remove a custom platform, or restore an overridden built-in driver.""" - platform = _normalize(platform) - if platform not in _registry: + name = _normalize(platform) + if name not in _registry: message = f"Unsupported platform: {platform}" raise DriverNotFoundError(message) - if isinstance(platform, Platform): - if _registry[platform] is _BUILTIN_DRIVERS[platform]: - message = f"Built-in platform {platform} is not overridden" - raise DriverNotFoundError(message) - _registry[platform] = _BUILTIN_DRIVERS[platform] + builtin = _BUILTIN_DRIVERS.get(name) + if builtin is None: + del _registry[name] + elif _registry[name] is builtin: + # Format the canonical name: pre-3.11 f-strings render a str-Enum + # member as its meaningless value string. + message = f"Built-in platform {name} is not overridden" + raise DriverNotFoundError(message) else: - del _registry[platform] + _registry[name] = builtin def get_registered_platforms() -> tuple[Platform | str, ...]: - """Return all registered platforms, built-in and custom.""" - return tuple(_registry) + """Return all registered platforms, built-in and custom. + + Names matching a `Platform` member are returned as members; custom names + are returned as canonical uppercase strings. + """ + return tuple(Platform.__members__.get(name, name) for name in _registry) def resolve_driver( diff --git a/tests/unit/test_registry.py b/tests/unit/test_registry.py index d73655e..592ece9 100644 --- a/tests/unit/test_registry.py +++ b/tests/unit/test_registry.py @@ -26,6 +26,10 @@ def _instantiate_rules() -> HConfigDriverRules: return HConfigDriverRules() +# The member's value string ("3"): a str-Enum artifact, not a platform name. +_CISCO_IOS_VALUE = str(Platform.CISCO_IOS.value) + + def _assert_custom_platform_works() -> None: driver = get_hconfig_driver("MY_NOS") assert isinstance(driver, _CustomDriver) @@ -116,3 +120,54 @@ def test_unregister_builtin_without_override_raises() -> None: """A built-in platform without an override cannot be unregistered.""" with pytest.raises(DriverNotFoundError, match="not overridden"): unregister_driver(Platform.CISCO_XR) + + +def test_register_platform_value_does_not_collide_with_builtin() -> None: + """A Platform member's value string is a distinct custom key (#284).""" + register_driver(_CISCO_IOS_VALUE, _CustomDriver) + try: + driver = get_hconfig_driver(Platform.CISCO_IOS) + finally: + unregister_driver(_CISCO_IOS_VALUE) + assert driver.__class__ is HConfigDriverCiscoIOS + + +def test_platform_value_lookup_raises() -> None: + """Platform member value strings are not platform names (#284).""" + with pytest.raises(DriverNotFoundError, match="Unsupported platform"): + get_hconfig_driver(_CISCO_IOS_VALUE) + + +def test_get_registered_platforms_includes_custom_names() -> None: + """Custom platforms are listed by their canonical uppercase name (#284).""" + register_driver("my_nos", _CustomDriver) + try: + assert [ + platform + for platform in get_registered_platforms() + if not isinstance(platform, Platform) + ] == ["MY_NOS"] + finally: + unregister_driver("my_nos") + + +def test_get_registered_platforms_returns_enum_members_for_builtins() -> None: + """Enum-known names are listed as Platform members, not strings (#284).""" + members = { + platform + for platform in get_registered_platforms() + if isinstance(platform, Platform) + } + assert members == set(Platform) + + +def test_platform_name_string_interchangeable_with_member() -> None: + """A Platform member and its name address the same registry entry (#284).""" + register_driver("cisco_ios", _CustomDriver) + try: + assert isinstance(get_hconfig_driver(Platform.CISCO_IOS), _CustomDriver) + finally: + unregister_driver(Platform.CISCO_IOS) + + driver = get_hconfig_driver(Platform.CISCO_IOS) + assert driver.__class__ is HConfigDriverCiscoIOS