Skip to content

Added validation for Gather filter supported properties - #423

Open
jeetugangwar11 wants to merge 5 commits into
CiscoDevNet:developfrom
jeetugangwar11:resource_manager_230726
Open

Added validation for Gather filter supported properties#423
jeetugangwar11 wants to merge 5 commits into
CiscoDevNet:developfrom
jeetugangwar11:resource_manager_230726

Conversation

@jeetugangwar11

Copy link
Copy Markdown
Collaborator

Proposed Changes

  • Define and document the supported state: gathered filter properties:
    • entity_name
    • pool_name
    • switches
    • resource
    • scope_type
    • pool_type
  • Use pool_name and switches as GET API query parameters.
  • Apply all six supported properties during local resource matching.
  • Do not use Lucene filtering because normalized entity matching for device-pair and link resources must be performed locally.
  • Reject unsupported gathered filter properties before making API requests.
  • Reject empty gathered filter items such as config: [{}].
  • Reject explicitly null supported properties, including when a null property is combined with another valid filter.
  • Preserve gather-all behavior when config is omitted or specified as an empty list.
  • Update module documentation with the supported filter matrix and validation behavior.
  • Add unit tests for supported, unsupported, empty, null-only, and mixed-null gathered filters.
  • Add invalid gathered-filter integration coverage to the root, BASE, iBGP, eBGP, and External resource-manager test suites.

Test Notes

The following validation was completed:

  • Focused gathered validation tests: 22 passed
  • Complete resource-manager unit suite: 291 passed
  • Parsed all five updated invalid_params.yaml files successfully.
  • Verified register names are unique within each integration test file.
  • Ran ansible-test sanity --test validate-modules --venv plugins/modules/nd_manage_resource_manager.py successfully.
  • Verified no VS Code diagnostics in the changed Python and YAML files.

Cisco Nexus Dashboard Version

Cisco Nexus Dashboard 4.3x

Related ND API Resource Category

  • analyze
  • infa
  • manage
  • onemanage
  • other

Resource category: Manage resource manager API (/manage/api/v1).

Checklist

  • Latest commit is rebased from develop with merge conflicts resolved
  • New or updates to documentation has been made accordingly
  • Assigned the proper reviewers

@allenrobel allenrobel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code review

Two inline comments below (one confirmed functional regression, one documentation-policy issue), plus one finding that lands on lines outside the diff:

  1. Stale "all fields optional" docstrings — the new validate_gathered_properties validator rejects a gathered filter item with zero active properties, so "all fields optional" is no longer accurate for gathered state. The module DOCUMENTATION was updated with the new "at least one supported property" constraint, but the model's own docstrings still make the old claim in two places: the class docstring (query/gathered: all fields optional (used as filters).) and the apply_state_validation docstring (all fields remain optional and serve as filters). Please update both to reflect the new constraint.

🤖 Generated with Claude Code

Comment thread plugins/modules/nd_manage_resource_manager.py Outdated
@mikewiebe

Copy link
Copy Markdown
Collaborator

I have completed my review and have no additional findings. Once the outstanding review comments from @allenrobel are resolved, this PR is good to merge from my perspective.

@allenrobel allenrobel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

The TODO is in the wrong format for an ND workaround since the version is not specified. But we can fix that later. Suggested fix:

# TODO(4.2.1) resource-manager-gathered-entity-order-lucene
# ND echoes canonical endpoint order for multi-switch entity names, so exact Lucene filters on
# user-supplied entity_name can miss matches; gathered filtering matches entity names locally instead.

@allenrobel

Copy link
Copy Markdown
Collaborator

@jeetugangwar11 when you get a chance later, below is the workaround note for the TODO(4.2.1) with the link back slug (id: field in the Markdown frontmatter) which links back to this note. Once we figure out how to make the MCP server that serves these notes shareable, your LLM will be able to reference this, and other, workarounds.

Thanks

---
id: resource-manager-gathered-entity-order-lucene
endpoints:
  - GET /api/v1/manage/fabrics/{fabricName}/resources
  - POST /api/v1/manage/fabrics/{fabricName}/resources
tags:
  - deviation
found: 4.2.1
fixed:
status: workaround
severity: medium
guidance: "Do not filter resources server-side by entityName (Lucene `filter=entityName:...`): ND echoes multi-switch entity names (devicePair/link scopes) in canonical endpoint order, so an exact match against the user-supplied tilde-separated serial order can silently miss resources. Fetch broader (poolName/switchId query params + pagination) and match entityName locally, order-insensitively on the tilde-separated segments."
---

# Summary

For multi-switch resources (`scopeType: devicePair` and `link`), `entityName` is a tilde-joined composite of switch serials plus an entity suffix (spec example: `FDO222419B7~FDO222419B8~vpc55`). ND normalizes these names to a **canonical endpoint order** on read: the order of the serial segments echoed by `GET /api/v1/manage/fabrics/{fabricName}/resources` is ND's, not necessarily the order the client supplied at allocation time.

The endpoint's `filter` query parameter is a Lucene **exact-match** filter (the spec documents it only as "Lucene format filter" with no field list). An exact `filter=entityName:<user-supplied-name>` therefore silently excludes resources whose stored name differs from the query only by endpoint order — the resource exists and would match after order-insensitive normalization, but the server-side filter never returns it.

## Impact

- Any client that pushes `entityName` filtering to the server via Lucene gets **silently incomplete results** for devicePair/link resources — no error, just missing rows.
- Clients must instead over-fetch and match locally, which costs extra transfer/pagination on large fabrics. In `cisco.nd`'s `nd_manage_resource_manager` (`state: gathered`), this is why `entity_name` is a local-match-only filter while only `poolName` and `switchId` are pushed as GET query parameters.
- Single-switch scopes (`fabric`, `device`, `deviceInterface`) have no serial-order ambiguity in `entityName`, but the collection matches locally across the board for consistency.

## Workaround

### Full

Fetch with the documented first-class query parameters (`poolName`, `switchId`) plus `max`/`offset` pagination to bound the result set, then match `entityName` client-side, comparing tilde-separated segments order-insensitively. This is what `nd_manage_resource_manager` does (see `_build_gathered_resource_criteria` in `plugins/module_utils/manage_resource_manager/nd_manage_resource_manager_resources.py`; workaround marker `TODO(4.2.1) resource-manager-gathered-entity-order-lucene`).

## Reproduction

Not yet independently lab-reproduced. Sourced from `cisco.nd` resource-manager module development (jeetugangwar11, PRs [[#333](https://github.com/CiscoDevNet/ansible-nd/pull/333)](https://github.com/CiscoDevNet/ansible-nd/pull/333) and [[#423](https://github.com/CiscoDevNet/ansible-nd/pull/423)](https://github.com/CiscoDevNet/ansible-nd/pull/423)): the original implementation carried the in-code observation that "ND may return canonical endpoint order for multi-switch resources, and some simple entity-only Lucene filters can exclude resources that should match after local normalization", and the module documents "Entity name matching is order-insensitive for tilde-separated serial numbers" as a consequence.

To reproduce on the lab (SITE1, 192.168.7.7):

1. Allocate a devicePair resource (e.g. a `vpcId`) naming the entity with serials in a chosen order: `SERIAL-B~SERIAL-A~vpc10`.
2. `GET /api/v1/manage/fabrics/{fabricName}/resources` and inspect the echoed `entityName` — expect canonical (reordered) serials.
3. `GET /api/v1/manage/fabrics/{fabricName}/resources?filter=entityName:SERIAL-B~SERIAL-A~vpc10` — expect the resource to be absent from the filtered response while present unfiltered.

Update this section (and consider promoting `tags` to include `bug`) once the lab run confirms steps 2–3.

## Expected behavior

Either the read echoes `entityName` exactly as allocated, or the server-side `filter` matches entity names order-insensitively — so a client can round-trip the name it allocated into a Lucene filter and find the resource.

## Actual behavior

The stored/echoed `entityName` is canonicalized, and the Lucene filter is exact, so a round-tripped user-supplied name can silently match nothing.

## Related

- [[[vPC per-peer VLAN fields collapse on read](https://github.com/CiscoDevNet/ansible-nd/pull/vPC%20per-peer%20VLAN%20fields%20collapse%20on%20read)]] — same family: ND normalizes the echo of what was written.

@jeetugangwar11

Copy link
Copy Markdown
Collaborator Author

Hi @allenrobel, I have made the changes for TODO, could you please have a look and verify if this looks good.

@allenrobel allenrobel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@mtarking mtarking changed the title [Jeet] Added validation for Gather filter supported properties. Added validation for Gather filter supported properties Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.0.0 Release 2.0.0 nac01 NaC ND release 0.0.1 ready for review Submitter is requesting a PR review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants