API: Admin endpoint cluster id filter for listing nodes#2641
Conversation
Previously endpoitn for listing nodes returned only nomad managed nodes (local cluster). Now it can return any nodes api handles for pre-selected cluster.
PR SummaryMedium Risk Overview Potential issues: this is a breaking signature change for any internal callers/tests using the generated Reviewed by Cursor Bugbot for commit b0f323e. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 9 Tests Failed:
View the full list of 12 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
Code Review
This pull request updates the GetNodes endpoint to support filtering by cluster ID, including necessary OpenAPI specification changes, generated client code updates, and logic modifications in the API handler and orchestrator. I have no feedback to provide as the changes correctly implement the requested functionality and follow the existing patterns.
There was a problem hiding this comment.
Straightforward admin filter addition following the existing cluster-fallback pattern used by sibling node endpoints.
Extended reasoning...
Overview
This PR adds an optional clusterID query parameter to the admin GET /nodes endpoint and replaces the prior IsNomadManaged() filter in Orchestrator.AdminNodes with a direct ClusterID comparison. The bulk of the diff is regenerated client/server scaffolding in api.gen.go and tests/integration/internal/api/generated.go from the openapi spec change; the only hand-written changes are a few lines in handlers/admin.go and orchestrator/admin.go, plus the spec.
Security risks
None significant. The endpoint is already gated by AdminTokenAuth. The new parameter is a UUID parsed via the generated runtime binder, so malformed input yields a 400. Behavior preservation: WithClusterFallback(nil) returns consts.LocalClusterID = uuid.Nil, which matches the ClusterID assigned to Nomad-managed nodes in nodemanager/node.go — so the default response when no clusterID is supplied is equivalent to the previous IsNomadManaged() filter.
Level of scrutiny
Low. Admin-only endpoint, small surface area, follows an established pattern already used by GetNodesNodeID and PostNodesNodeID in the same file. The generated code is mechanical oapi-codegen output. No auth, crypto, or data-mutation logic is touched.
Other factors
No outstanding reviewer comments. The bug hunting system found no issues. The only timeline entry is a Cursor bot summary placeholder.
Adds support to query nodes in non-default clusters
Adds support to query nodes in non-default clusters