Skip to content

Add Smart Endpoints framework for ND API v1#186

Merged
lhercot merged 45 commits intond42_integrationfrom
nd42_smart_endpoints
Mar 11, 2026
Merged

Add Smart Endpoints framework for ND API v1#186
lhercot merged 45 commits intond42_integrationfrom
nd42_smart_endpoints

Conversation

@allenrobel
Copy link
Collaborator

@allenrobel allenrobel commented Mar 3, 2026

Summary

Adds the Smart Endpoints framework — a type-safe, Pydantic-validated approach to constructing ND REST API endpoint paths and query parameters. This PR is one of three companion PRs targeting nd42_integration:

PR Branch Contents Merge order
#185 nd42_rest_send RestSend framework, enums.py, shared test infrastructure 1st
#184 nd42_logging Log class and logging config 2nd
This PR nd42_smart_endpoints Smart Endpoints framework 3rd

Note: Tests depend on enums.py, pydantic_compat.py, and shared test infrastructure from nd42_rest_send (#185), and on log.py from nd42_logging (#184). Tests will not run until all three branches are merged into nd42_integration.


Plugin files added

Base classes (plugins/module_utils/endpoints/)

File Description
base_path.py ApiPath — string enum providing type-safe base API path constants shared across all endpoint versions
endpoint_mixins.py Reusable endpoint mixins (e.g. LoginIdMixin for endpoints requiring a login ID)
query_params.py EndpointQueryParams, LuceneQueryParams, CompositeQueryParams — Pydantic models for structured, validated query string construction

ND API v1 endpoints (plugins/module_utils/endpoints/v1/)

File Description
base_paths_infra.py Base path constants for the ND /api/v1/infra namespace
base_paths_manage.py Base path constants for the ND /api/v1/manage namespace
infra_aaa.py AAA (login/logout) endpoints
infra_clusterhealth.py Cluster health endpoints
infra_login.py Login endpoint (EpInfraLoginPost) for /api/v1/infra/login
manage_switches.py Switch management endpoint with Lucene query support

Unit tests added

File Covers
test_base_path.py ApiPath base class
test_base_paths_infra.py Infra base paths
test_base_paths_manage.py Manage base paths
test_endpoint_mixins.py LoginIdMixin and other mixins
test_endpoints_api_v1_infra_aaa.py AAA endpoints
test_endpoints_api_v1_infra_clusterhealth.py Cluster health endpoints
test_endpoints_api_v1_infra_login.py Login endpoint
test_endpoints_api_v1_manage_switches.py Switch management endpoint
test_query_params.py All query parameter models

Dependencies

Merge order

  1. Merge nd42_rest_send (Add RestSend framework, enums, and shared unit test infrastructure #185) → nd42_integration
  2. Merge nd42_logging (Add Log class and logging config #184) → nd42_integration
  3. Merge this PR → nd42_integration

Test plan

  • After merging all three PRs into nd42_integration: python -m pytest tests/unit/module_utils/endpoints/ passes cleanly
  • tox -e linters passes (black, isort, pylint, mypy)
  • ansible-test sanity --docker passes

🤖 Generated with Claude Code

Base classes:
- plugins/module_utils/ep/base_path.py: ApiPath base class for type-safe
  endpoint path construction
- plugins/module_utils/ep/base_paths_ndfc.py: NDFC base path constants
- plugins/module_utils/ep/endpoint_mixins.py: Reusable endpoint mixins
  (e.g. LoginIdMixin)
- plugins/module_utils/ep/query_params.py: EndpointQueryParams,
  LuceneQueryParams, CompositeQueryParams for structured query string building

ND API v1 endpoints:
- plugins/module_utils/ep/v1/base_paths_infra.py: Infra API base paths
- plugins/module_utils/ep/v1/base_paths_manage.py: Manage API base paths
- plugins/module_utils/ep/v1/ep_infra_aaa.py: AAA (login) endpoint
- plugins/module_utils/ep/v1/ep_infra_clusterhealth.py: Cluster health endpoint
- plugins/module_utils/ep/v1/ep_manage_switches.py: Switch management endpoint

Design documentation:
- plugins/module_utils/ep/ANALYSIS_COMPLEXITY.md
- plugins/module_utils/ep/ANALYSIS_ENDPOINT_DESIGN.md
- plugins/module_utils/ep/ANALYSIS_TYPE_SAFETY.md

Unit tests:
- tests/unit/module_utils/ep/test_base_path.py
- tests/unit/module_utils/ep/test_base_paths_infra.py
- tests/unit/module_utils/ep/test_base_paths_manage.py
- tests/unit/module_utils/ep/test_endpoint_mixins.py
- tests/unit/module_utils/ep/test_ep_api_v1_infra_aaa.py
- tests/unit/module_utils/ep/test_ep_api_v1_infra_clusterhealth.py
- tests/unit/module_utils/ep/test_ep_api_v1_manage_switches.py
- tests/unit/module_utils/ep/test_query_params.py

Note: tests depend on enums.py, pydantic_compat.py, and shared test
infrastructure (common_utils, etc.) from nd42_rest_send, and on log.py
from nd42_logging. Tests will not run until all branches are merged
into nd42_integration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
allenrobel and others added 10 commits March 3, 2026 07:30
1. Remove /mso, /appcenter/*, /login paths

TODO: Need to add /api/v1/infra/login under infra endpoints.
Removed the following from all files:

# -*- coding: utf-8 -*-
- plugins/module_utils/ep/ → plugins/module_utils/endpoints/
- v1/ep_infra_aaa.py → v1/infra_aaa.py
- v1/ep_infra_clusterhealth.py → v1/infra_clusterhealth.py
- v1/ep_manage_switches.py → v1/manage_switches.py
- tests/unit/module_utils/ep/ → tests/unit/module_utils/endpoints/
- test_ep_api_v1_*.py → test_endpoints_api_v1_*.py
- Update all internal imports to reflect new paths

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the test_ep_ prefix in all test function names within the
three ep-specific test files to match the renamed file and directory.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ansible sanity tests use a regex to verify the __metaclass__ = type
declaration and do not expect an inline comment. Move the pylint
disable/enable directives to separate lines.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
allenrobel and others added 5 commits March 4, 2026 08:50
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pydantic_compat was moved into module_utils/commmon in nd42_rest_send branch.  nd42_smart_endpoints and nd42_rest_send will be merged into a common integration branch shortly, so am updating at least some of the imports that are known to have changed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Forgot to add this in the earlier commit.
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

The reviewer correctly identified that typing CompositeQueryParams._param_groups
as list[Union[EndpointQueryParams, LuceneQueryParams]] defeats the purpose of
having a QueryParams interface — every new QueryParams type would require
updating the Union, which is exactly what an interface is meant to avoid.

The fix replaces the QueryParams ABC with a typing.Protocol. This gives us
structural (duck) typing: any class that implements to_query_string() and
is_empty() automatically satisfies the QueryParams protocol without needing
to inherit from it. This is ideal here because EndpointQueryParams and
LuceneQueryParams already inherit from Pydantic's BaseModel and cannot
also inherit from an ABC cleanly.

Key details:
- Protocol is used only for static type hints, not at runtime
- No @runtime_checkable decorator, so no risk of hasattr() triggering
  property getters (a known issue in Python 3.8-3.11)
- The isinstance(param_group, LuceneQueryParams) check in
  CompositeQueryParams.to_query_string() remains — it handles the
  differing method signatures (LuceneQueryParams.to_query_string accepts
  url_encode, EndpointQueryParams does not)
- Triple try/except for Protocol import maintains Python 3.7 compat
  required by ansible-test sanity

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

Introduces NDEndpointBaseModel (endpoints/base.py), an abstract base
class combining Pydantic BaseModel with ABC. It centralizes:

- model_config (ConfigDict with validate_assignment=True)
- api_version (default "v1")
- min_controller_version (default "3.0.0")
- Abstract properties: path and verb (enforced by ABC)

All existing endpoint classes now inherit from NDEndpointBaseModel
instead of bare BaseModel, removing duplicated model_config,
api_version, and min_controller_version fields from every endpoint.

This aligns with Gaspard's NDBaseEndpoint pattern and makes it
easier to add global endpoint behavior (e.g., set_identifiers)
in his PR without touching every endpoint file.

## Notes for Ansible module developers

- New endpoints MUST inherit from NDEndpointBaseModel (imported from
  endpoints.base), not from BaseModel directly
- Every concrete (instantiable) endpoint class MUST define:
  - class_name: a Literal field identifying the class
  - path: a @Property returning the URL path string
  - verb: a @Property returning an HttpVerbEnum value
  Omitting any of these will raise TypeError at instantiation
- Intermediate base classes (e.g., _EpInfraAaaLocalUsersBase) may
  leave verb abstract — the ABC will prevent direct instantiation
  while allowing leaf classes to provide the implementation
- Import path: use `from ...endpoints.base import NDEndpointBaseModel`
- model_config, api_version, and min_controller_version are inherited
  automatically — do NOT redeclare them in endpoint classes
- COMMON_CONFIG module-level constants are no longer needed and have
  been removed

## Unit test impact

No test changes required — all existing tests pass unchanged since
the public interface (class_name, path, verb, api_version, etc.)
is identical.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

ClusterHealthConfigEndpointParams and ClusterHealthStatusEndpointParams
were redeclaring fields (cluster_name, health_category, node_name)
that already exist in the mixin classes. This violates the DRY principle
and risks the field definitions drifting out of sync.

Now these classes compose the existing ClusterNameMixin,
HealthCategoryMixin, and NodeNameMixin via multiple inheritance,
eliminating the duplicated Field declarations.

Tests are updated to be query-parameter-order-independent using
set comparisons, since Pydantic's field ordering with multiple
inheritance depends on MRO and should not be relied upon. HTTP
query parameter order is semantically irrelevant per RFC 3986.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

The BasePath method names included redundant prefixes (nd_infra_*,
nd_manage_*) when the class and module already convey the API scope.

base_paths_infra.py:
- nd_infra() -> path()
- nd_infra_aaa() -> aaa()
- nd_infra_clusterhealth() -> clusterhealth()

base_paths_manage.py:
- nd_manage() -> path()
- nd_manage_inventory() -> inventory()

This gives a consistent interface across both BasePath classes:
BasePath.path() builds the root, and short convenience methods
(aaa, clusterhealth, inventory) delegate to it.

Updated all callers in source and test files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

The individual mixin getter/setter/default tests were testing Pydantic's
Field behavior (defaults, assignment), not our code. Mixins contain zero
custom logic, so these tests added no meaningful coverage.

Retained tests:
- test_endpoint_mixins_00220: Validates our max_length constraint
  configuration on FabricNameMixin is correct (tests our config, not
  Pydantic)
- test_endpoint_mixins_01100: Validates mixin composition via multiple
  inheritance works correctly (tests our design pattern)

Removed 20 tests that verified Pydantic's own default/getter/setter
behavior across all mixin classes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

This is an entirely new REST API layer with no existing consumers, so
there is nothing to be backward compatible with. The module-level
constants (ND_ANALYZE_API, ND_INFRA_API, ND_MANAGE_API, ND_ONEMANAGE_API)
were aliases for ApiPath enum values that added no value and implied a
deprecation story that doesn't apply.

Changes:
- Remove all 4 legacy constants and the "Backward Compatibility"
  docstring section from base_path.py
- Remove unused Final/TYPE_CHECKING imports from base_path.py
- Rewrite test_base_path.py to test only the ApiPath enum, removing
  references to non-existent members (LOGIN, MSO, NDFC) that were
  causing import errors
- Update test_base_paths_infra.py and test_base_paths_manage.py to
  remove legacy constant imports and assertions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Per reviewer feedback, name endpoint files by specific API endpoint
rather than broad API area. This avoids future merge conflicts and
keeps files small as new AAA endpoints are added.

Ref: CiscoDevNet#186 (comment)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Pydantic's @field_validator registers by field name, not method name,
so the underscore prefix works and better reflects internal-only usage.

Ref: CiscoDevNet#186 (comment)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

The reviewer correctly identified that typing CompositeQueryParams._param_groups
as list[Union[EndpointQueryParams, LuceneQueryParams]] defeats the purpose of
having a QueryParams interface — every new QueryParams type would require
updating the Union, which is exactly what an interface is meant to avoid.

The fix replaces the QueryParams ABC with a typing.Protocol. This gives us
structural (duck) typing: any class that implements to_query_string() and
is_empty() automatically satisfies the QueryParams protocol without needing
to inherit from it. This is ideal here because EndpointQueryParams and
LuceneQueryParams already inherit from Pydantic's BaseModel and cannot
also inherit from an ABC cleanly.

Key details:
- Protocol is used only for static type hints, not at runtime
- No @runtime_checkable decorator, so no risk of hasattr() triggering
  property getters (a known issue in Python 3.8-3.11)
- The isinstance(param_group, LuceneQueryParams) check in
  CompositeQueryParams.to_query_string() remains — it handles the
  differing method signatures (LuceneQueryParams.to_query_string accepts
  url_encode, EndpointQueryParams does not)
- Triple try/except for Protocol import maintains Python 3.7 compat
  required by ansible-test sanity

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

Introduces NDEndpointBaseModel (endpoints/base.py), an abstract base
class combining Pydantic BaseModel with ABC. It centralizes:

- model_config (ConfigDict with validate_assignment=True)
- api_version (default "v1")
- min_controller_version (default "3.0.0")
- Abstract properties: path and verb (enforced by ABC)

All existing endpoint classes now inherit from NDEndpointBaseModel
instead of bare BaseModel, removing duplicated model_config,
api_version, and min_controller_version fields from every endpoint.

This aligns with Gaspard's NDBaseEndpoint pattern and makes it
easier to add global endpoint behavior (e.g., set_identifiers)
in his PR without touching every endpoint file.

## Notes for Ansible module developers

- New endpoints MUST inherit from NDEndpointBaseModel (imported from
  endpoints.base), not from BaseModel directly
- Every concrete (instantiable) endpoint class MUST define:
  - class_name: a Literal field identifying the class
  - path: a @Property returning the URL path string
  - verb: a @Property returning an HttpVerbEnum value
  Omitting any of these will raise TypeError at instantiation
- Intermediate base classes (e.g., _EpInfraAaaLocalUsersBase) may
  leave verb abstract — the ABC will prevent direct instantiation
  while allowing leaf classes to provide the implementation
- Import path: use `from ...endpoints.base import NDEndpointBaseModel`
- model_config, api_version, and min_controller_version are inherited
  automatically — do NOT redeclare them in endpoint classes
- COMMON_CONFIG module-level constants are no longer needed and have
  been removed

## Unit test impact

No test changes required — all existing tests pass unchanged since
the public interface (class_name, path, verb, api_version, etc.)
is identical.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

ClusterHealthConfigEndpointParams and ClusterHealthStatusEndpointParams
were redeclaring fields (cluster_name, health_category, node_name)
that already exist in the mixin classes. This violates the DRY principle
and risks the field definitions drifting out of sync.

Now these classes compose the existing ClusterNameMixin,
HealthCategoryMixin, and NodeNameMixin via multiple inheritance,
eliminating the duplicated Field declarations.

Tests are updated to be query-parameter-order-independent using
set comparisons, since Pydantic's field ordering with multiple
inheritance depends on MRO and should not be relied upon. HTTP
query parameter order is semantically irrelevant per RFC 3986.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

The BasePath method names included redundant prefixes (nd_infra_*,
nd_manage_*) when the class and module already convey the API scope.

base_paths_infra.py:
- nd_infra() -> path()
- nd_infra_aaa() -> aaa()
- nd_infra_clusterhealth() -> clusterhealth()

base_paths_manage.py:
- nd_manage() -> path()
- nd_manage_inventory() -> inventory()

This gives a consistent interface across both BasePath classes:
BasePath.path() builds the root, and short convenience methods
(aaa, clusterhealth, inventory) delegate to it.

Updated all callers in source and test files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

The individual mixin getter/setter/default tests were testing Pydantic's
Field behavior (defaults, assignment), not our code. Mixins contain zero
custom logic, so these tests added no meaningful coverage.

Retained tests:
- test_endpoint_mixins_00220: Validates our max_length constraint
  configuration on FabricNameMixin is correct (tests our config, not
  Pydantic)
- test_endpoint_mixins_01100: Validates mixin composition via multiple
  inheritance works correctly (tests our design pattern)

Removed 20 tests that verified Pydantic's own default/getter/setter
behavior across all mixin classes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Address review comment:
CiscoDevNet#186 (comment)

This is an entirely new REST API layer with no existing consumers, so
there is nothing to be backward compatible with. The module-level
constants (ND_ANALYZE_API, ND_INFRA_API, ND_MANAGE_API, ND_ONEMANAGE_API)
were aliases for ApiPath enum values that added no value and implied a
deprecation story that doesn't apply.

Changes:
- Remove all 4 legacy constants and the "Backward Compatibility"
  docstring section from base_path.py
- Remove unused Final/TYPE_CHECKING imports from base_path.py
- Rewrite test_base_path.py to test only the ApiPath enum, removing
  references to non-existent members (LOGIN, MSO, NDFC) that were
  causing import errors
- Update test_base_paths_infra.py and test_base_paths_manage.py to
  remove legacy constant imports and assertions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jeetugangwar11 pushed a commit to jeetugangwar11/ansible-nd that referenced this pull request Mar 11, 2026
Per reviewer feedback, name endpoint files by specific API endpoint
rather than broad API area. This avoids future merge conflicts and
keeps files small as new AAA endpoints are added.

Ref: CiscoDevNet#186 (comment)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
allenrobel and others added 2 commits March 11, 2026 06:04
Split clusterhealth.py into clusterhealth_config.py and
clusterhealth_status.py to enforce one endpoint class per file.
Update test imports to reference the new module paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mikewiebe
mikewiebe previously approved these changes Mar 11, 2026
Copy link
Collaborator

@mikewiebe mikewiebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The source file aaa_local_users.py was removed in 00fda96 but
the associated test file was not deleted at that time.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
allenrobel and others added 3 commits March 11, 2026 07:03
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The try/except fallback chain for Protocol was unnecessary since
python_requires: controller and requires_ansible >= 2.16.0 guarantee
Python >= 3.10, where typing.Protocol is always available.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The metaclass-boilerplate sanity test has been removed from
ansible-core 2.19. With python_requires: controller in tests/config.yml
and requires_ansible >= 2.16.0 (Python >= 3.10), all classes are
new-style by default and __metaclass__ = type is dead code. The
associated pylint disable/enable directives are also removed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@lhercot lhercot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lhercot lhercot merged commit 756bdcc into nd42_integration Mar 11, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants