diff --git a/CHANGELOG.md b/CHANGELOG.md index ebc8476..6cac205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to netbox-super-cli are tracked here. Format follows [Keep a ## Unreleased +### Fixed + +- **Plugin endpoints get one command group per resource** ([#202]). NetBox tags + every plugin endpoint `plugins`, so all of a plugin's endpoints were merged + into one `nsc plugins ` resource whose verbs overwrote each other. For + example, `nsc plugins branching list` listed *changes* while `create` created + *branches*, and some endpoints were unreachable. Plugin commands now follow the + path. **Command paths change:** + `nsc plugins branching list` → `nsc branching changes list`, and + `nsc branching branches list|get|create|merge-create|…`, `nsc branching branch-events list`, etc. + Cached command models are rebuilt automatically. + ## v1.8.7 — 2026-10-24 Patch release. A literal `-o` used as another option's value is no longer read as the output flag. @@ -193,6 +205,7 @@ notices, and a fix for `nsc login` reporting the wrong user. [#189]: https://github.com/thomaschristory/netbox-super-cli/issues/189 [#192]: https://github.com/thomaschristory/netbox-super-cli/issues/192 [#196]: https://github.com/thomaschristory/netbox-super-cli/issues/196 +[#202]: https://github.com/thomaschristory/netbox-super-cli/issues/202 [#171]: https://github.com/thomaschristory/netbox-super-cli/issues/171 [#179]: https://github.com/thomaschristory/netbox-super-cli/issues/179 [#173]: https://github.com/thomaschristory/netbox-super-cli/issues/173 diff --git a/docs/guides/working-with-plugins.md b/docs/guides/working-with-plugins.md index 2028fb3..929f8d3 100644 --- a/docs/guides/working-with-plugins.md +++ b/docs/guides/working-with-plugins.md @@ -6,8 +6,19 @@ automatically — no per-plugin code in `nsc`**. ## How plugin endpoints surface -If a plugin registers `tags: [my_plugin]` and a path `/api/plugins/my-plugin/widgets/`, -you get: +Each `/api/plugins///` path becomes its own resource. NetBox +tags every plugin endpoint with the generic `plugins` tag; `nsc` then groups +them under the plugin's URL segment instead. For example, NetBox Branching's +`/api/plugins/branching/branches/` and `/api/plugins/branching/changes/` give: + +```sh +nsc branching branches list +nsc branching branches merge-create 3 --apply +nsc branching changes list --branch +``` + +A plugin that sets its own tag keeps it. With `tags: [my_plugin]` and a path +`/api/plugins/my-plugin/widgets/`, you get: ```sh nsc my_plugin widgets list @@ -15,6 +26,9 @@ nsc my_plugin widgets get 7 nsc my_plugin widgets create --field name=foo --apply ``` +A built-in command with the same name (for example `branch`) takes precedence +over a plugin group. + Verbs are derived from HTTP method + `operationId` exactly as for core endpoints. Custom action endpoints (`POST /api/plugins/my-plugin/widgets/{id}/calibrate/`) become e.g. `nsc my_plugin widgets calibrate 7 --apply`. diff --git a/nsc/builder/build.py b/nsc/builder/build.py index 2c27316..d63d05e 100644 --- a/nsc/builder/build.py +++ b/nsc/builder/build.py @@ -52,6 +52,7 @@ ) # api / / = at least 3 non-empty path parts _MIN_PATH_PARTS = 3 +_GENERIC_PLUGIN_TAG = "plugins" def build_command_model(loaded: LoadedSchema) -> CommandModel: @@ -118,7 +119,7 @@ def _assimilate( if tag_name is None: return # untagged operations are skipped intentionally - resource_name, _ = _resource_from_path(path, tag_name) + tag_name, resource_name, _ = _route(path, tag_name) if resource_name is None: return # paths that don't match the /api///... shape @@ -136,6 +137,25 @@ def _assimilate( _attach(resource, _classify(http_method, path), op) +def _route(path: str, spec_tag: str) -> tuple[str, str | None, bool]: + """Return (tag, resource_name, is_collection_path) for an operation's path. + + Issue #202: NetBox tags every plugin endpoint with the generic `plugins`; + grouping on it merged all of a plugin's endpoints into one resource whose + CRUD slots overwrote each other. For `/api/plugins///...` + the resource comes from the path, and the generic tag becomes ``. + """ + parts = [p for p in path.split("/") if p] + if len(parts) < _MIN_PATH_PARTS or parts[:2] != ["api", _GENERIC_PLUGIN_TAG]: + resource, is_collection = _resource_from_path(path, spec_tag) + return spec_tag, resource, is_collection + tag = parts[2] if spec_tag == _GENERIC_PLUGIN_TAG else spec_tag + # `/api/plugins//` is the plugin's root view, not a resource. + if len(parts) == _MIN_PATH_PARTS or _PARAM_SEGMENT.match(parts[3]): + return tag, None, False + return tag, parts[3], len(parts) == _MIN_PATH_PARTS + 1 + + def _resource_from_path(path: str, tag: str) -> tuple[str | None, bool]: """Return (resource_name, is_collection_path). @@ -153,12 +173,8 @@ def _resource_from_path(path: str, tag: str) -> tuple[str | None, bool]: # Expect: api / / / [...] if len(parts) < _MIN_PATH_PARTS: return None, False - # Some plugin endpoints have nested tags (e.g., ['plugins', '']). - # The tag string from the spec is authoritative; just find . - # Strategy: skip leading non-parameter segments until we've consumed the - # tag's words and reached a resource segment. - # Easier heuristic that works for NetBox: the resource is the first - # segment after the segment that begins the tag's path-form. + # Plugin paths are handled by `_route`. Otherwise the resource is + # the first segment after the segment that begins the tag's path-form. tag_form = tag.replace(" ", "-").lower() try: anchor = parts.index(tag_form) @@ -438,8 +454,7 @@ def _build_fk_resources(doc: OpenAPIDocument) -> dict[str, FkResourceRef]: get_op = doc.paths[path].get if get_op is None or not get_op.tags: continue - tag_name = get_op.tags[0] - resource_name, is_collection = _resource_from_path(path, tag_name) + tag_name, resource_name, is_collection = _route(path, get_op.tags[0]) if resource_name is None or not is_collection: continue serializer = _list_serializer_name(get_op, doc) diff --git a/nsc/model/command_model.py b/nsc/model/command_model.py index 284b1f0..8c12d02 100644 --- a/nsc/model/command_model.py +++ b/nsc/model/command_model.py @@ -46,7 +46,7 @@ class PrimitiveType(StrEnum): # Bumped whenever the builder changes the shape of a CommandModel in a way that # makes older cached models incomplete (not just a schema-body change). The cache # rebuilds entries stamped with an older version. See nsc/cache/store.py. -MODEL_FORMAT_VERSION = 1 +MODEL_FORMAT_VERSION = 2 class _Frozen(BaseModel): diff --git a/tests/builder/test_plugin_paths.py b/tests/builder/test_plugin_paths.py new file mode 100644 index 0000000..9e5e45e --- /dev/null +++ b/tests/builder/test_plugin_paths.py @@ -0,0 +1,150 @@ +"""Plugin endpoints get one resource per `/api/plugins///` path (#202).""" + +from __future__ import annotations + +import json +from typing import Any + +from nsc.builder.build import build_command_model +from nsc.model.command_model import CommandModel, FkResourceRef +from nsc.schema.hashing import canonical_sha256 +from nsc.schema.loader import LoadedSchema +from nsc.schema.models import OpenAPIDocument + + +def _op(operation_id: str, tag: str) -> dict[str, Any]: + return {"operationId": operation_id, "tags": [tag], "responses": {"200": {"description": "OK"}}} + + +def _build(paths: dict[str, dict[str, Any]]) -> CommandModel: + doc_dict: dict[str, Any] = { + "openapi": "3.0.3", + "info": {"title": "Test", "version": "0.0.1"}, + "paths": paths, + } + body = json.dumps(doc_dict).encode() + doc = OpenAPIDocument.model_validate(doc_dict) + loaded = LoadedSchema(source="", body=body, hash=canonical_sha256(body), document=doc) + return build_command_model(loaded) + + +def _branching_paths(tag: str = "plugins") -> dict[str, dict[str, Any]]: + # Shape of netbox-branching 1.2 on NetBox 4.7: every path is tagged `plugins`. + base = "/api/plugins/branching" + return { + f"{base}/branches/": { + "get": _op("plugins_branching_branches_list", tag), + "post": _op("plugins_branching_branches_create", tag), + }, + f"{base}/branches/{{id}}/": { + "get": _op("plugins_branching_branches_retrieve", tag), + "patch": _op("plugins_branching_branches_partial_update", tag), + "delete": _op("plugins_branching_branches_destroy", tag), + }, + f"{base}/branches/{{id}}/merge/": { + "post": _op("plugins_branching_branches_merge_create", tag) + }, + f"{base}/changes/": {"get": _op("plugins_branching_changes_list", tag)}, + f"{base}/changes/{{id}}/": {"get": _op("plugins_branching_changes_retrieve", tag)}, + f"{base}/branch-events/": {"get": _op("plugins_branching_branch_events_list", tag)}, + f"{base}/branchable-models/": {"get": _op("plugins_branching_branchable_models_list", tag)}, + } + + +def test_generic_plugins_tag_becomes_the_plugin_name() -> None: + model = _build(_branching_paths()) + assert "plugins" not in model.tags + assert set(model.tags["branching"].resources) == { + "branches", + "changes", + "branch-events", + "branchable-models", + } + + +def test_plugin_resources_keep_their_own_operations() -> None: + resources = _build(_branching_paths()).tags["branching"].resources + branches = resources["branches"] + assert branches.list_op is not None + assert branches.list_op.path == "/api/plugins/branching/branches/" + assert branches.create_op is not None + assert branches.create_op.path == "/api/plugins/branching/branches/" + assert branches.get_op is not None + assert branches.get_op.path == "/api/plugins/branching/branches/{id}/" + assert [a.path for a in branches.custom_actions] == [ + "/api/plugins/branching/branches/{id}/merge/" + ] + changes = resources["changes"] + assert changes.list_op is not None + assert changes.list_op.path == "/api/plugins/branching/changes/" + assert changes.create_op is None + + +def test_plugin_with_its_own_tag_keeps_it() -> None: + model = _build( + { + "/api/plugins/my-plugin/widgets/": { + "get": _op("plugins_my_plugin_widgets_list", "my_plugin") + } + } + ) + widgets = model.tags["my_plugin"].resources["widgets"] + assert widgets.list_op is not None + assert widgets.list_op.path == "/api/plugins/my-plugin/widgets/" + + +def test_two_plugins_under_the_generic_tag_stay_separate() -> None: + model = _build( + { + "/api/plugins/branching/changes/": { + "get": _op("plugins_branching_changes_list", "plugins") + }, + "/api/plugins/netbox-dns/zones/": { + "get": _op("plugins_netbox_dns_zones_list", "plugins") + }, + } + ) + assert set(model.tags["branching"].resources) == {"changes"} + assert set(model.tags["netbox-dns"].resources) == {"zones"} + + +def test_plugin_root_view_is_skipped() -> None: + model = _build({"/api/plugins/branching/": {"get": _op("plugins_branching_root", "plugins")}}) + assert model.tags == {} + + +def test_plugin_list_serializer_is_indexed_to_its_resource() -> None: + list_op = _op("plugins_branching_branches_list", "plugins") + list_op["responses"] = { + "200": { + "description": "OK", + "content": { + "application/json": {"schema": {"$ref": "#/components/schemas/PaginatedBranchList"}} + }, + } + } + doc_dict: dict[str, Any] = { + "openapi": "3.0.3", + "info": {"title": "Test", "version": "0.0.1"}, + "paths": {"/api/plugins/branching/branches/": {"get": list_op}}, + "components": { + "schemas": { + "PaginatedBranchList": { + "type": "object", + "properties": { + "results": { + "type": "array", + "items": {"$ref": "#/components/schemas/Branch"}, + } + }, + }, + "Branch": {"type": "object", "properties": {"id": {"type": "integer"}}}, + } + }, + } + body = json.dumps(doc_dict).encode() + doc = OpenAPIDocument.model_validate(doc_dict) + model = build_command_model( + LoadedSchema(source="", body=body, hash=canonical_sha256(body), document=doc) + ) + assert model.fk_resources["Branch"] == FkResourceRef(tag="branching", resource="branches") diff --git a/tests/e2e/test_branching.py b/tests/e2e/test_branching.py index 5f6d644..6c3472b 100644 --- a/tests/e2e/test_branching.py +++ b/tests/e2e/test_branching.py @@ -144,3 +144,32 @@ def test_conflict_on_merge(run_nsc: RunNsc, netbox_client: httpx2.Client, branch assert r.returncode == 0, r.stderr detail = netbox_client.get(f"/api/extras/tags/{tag_id}/").json() assert detail["description"] == "from-branch" + + +@pytest.mark.usefixtures("clean_tags") +def test_plugin_endpoints_and_subcommand_branch_filter(run_nsc: RunNsc, branch: str) -> None: + """#202: each plugin endpoint is its own resource. #170/#180: a subcommand's + `--branch` query filter does not switch the invocation into a branch.""" + tag = _name("tag") + r = run_nsc( + "extras", + "tags", + "create", + "--field", + f"name={tag}", + "--field", + f"slug={tag}", + "--apply", + env={"NSC_BRANCH": branch}, + ) + assert r.returncode == 0, r.stderr + + r = run_nsc("branching", "branches", "list", "--output", "json") + assert r.returncode == 0, r.stderr + branches = {b["name"]: b["schema_id"] for b in r.stdout_json()} # type: ignore[union-attr] + assert branch in branches + + r = run_nsc("branching", "changes", "list", "--branch", branches[branch], "--output", "json") + assert r.returncode == 0, r.stderr + assert "⎇" not in r.stderr + assert [c["object_repr"] for c in r.stdout_json()] == [tag] # type: ignore[union-attr]