Migrate org.apache.solr.cli tools from V1 to V2 APIs

[epugh]

Replaces legacy V1 admin API calls (CollectionAdminRequest, CoreAdminRequest) in the bin/solr CLI tools with generated V2 OpenAPI SolrJ classes. Also adds a new proper SolrJ V2 endpoint for GET /cluster/nodes that was previously missing, eliminating the need for a GenericV2SolrRequest workaround.

I used Copilot for this, and here is the agent chat: https://github.com/epugh/solr/tasks/3c2f15c6-c0c7-4e39-b6b6-1cd51f8e7aa4

Migrated calls

Class	Method	Before → After
CLIUtils	safeCheckCollectionExists	CollectionAdminRequest.List → CollectionsApi.ListCollections
CreateTool	createCore	CoreAdminRequest.createCore → CoresApi.CreateCore
CreateTool	createCollection	CollectionAdminRequest.createCollection → CollectionsApi.CreateCollection
DeleteTool	deleteCollection	CollectionAdminRequest.deleteCollection → CollectionsApi.DeleteCollection
DeleteTool	deleteCore	CoreAdminRequest.Unload → CoresApi.UnloadCore
StatusTool	getCloudStatus	CollectionAdminRequest.ClusterStatus → ClusterApi.ListClusterNodes + CollectionsApi.ListCollections

Verbose output restored via Jackson

The old V1 code serialized NamedList responses with noggit's JSONWriter. V2 response POJOs are Jackson-annotated, so verbose output now uses JacksonContentWriter.DEFAULT_MAPPER.writerWithDefaultPrettyPrinter() — actually cleaner output since the responses are already proper JSON objects:

// CreateTool, DeleteTool — after req.process():
if (isVerbose() && response != null) {
  echo(JacksonContentWriter.DEFAULT_MAPPER
      .writerWithDefaultPrettyPrinter()
      .writeValueAsString(response));
}

New GET /cluster/nodes V2 API (SolrJ support)

StatusTool previously fell back to GenericV2SolrRequest because SolrJ had no typed class for this endpoint. The full API pipeline was wired up:

• ListClusterNodesResponse — new model with Set<String> nodes
• ListClusterNodesApi — new endpoint interface at @Path("/cluster/nodes")
• ListClusterNodes — new Jersey implementation registered in CollectionsHandler
• ClusterApi.ListClusterNodes — generated SolrJ request class (replaces GenericV2SolrRequest)

// Before
NamedList<Object> resp = solrClient.request(
    new GenericV2SolrRequest(SolrRequest.METHOD.GET, "/cluster/nodes", SolrRequestType.ADMIN));
var liveNodes = (Collection<?>) resp.get("nodes");

// After
var resp = new ClusterApi.ListClusterNodes().process(solrClient);
var liveNodes = resp.nodes; // Set<String>

Not migrated

Snapshot tools (SnapshotCreateTool, SnapshotDeleteTool, etc.) are intentionally excluded as candidates for removal.

In fact, we may just remove them...?

Tests

Existing tests + bats tests all pass.

[dsmiley]

looks good; just some minor feedback

[gerlowskija]

Right now, Solr allows users to disable the v2 APIs with a boolean sysprop: disable.v2.api

I'm 100% in favor of using the v2 APIs in more places - we desperately need that dog-fooding on them and this is a great way to do that. But if we're going to do this then we need to rip out that sysprop.

Ideally that'd happen before this PR gets merged, just to make sure we don't forget about it and accidentally release something where the bin/solr tools could be effectively bricked on any Solr deploys that use the sysprop. But I'd also be OK with filing a "Blocker" JIRA ticket, or a promise to tackle it soon, or something similar.

[epugh]

Right now, Solr allows users to disable the v2 APIs with a boolean sysprop: disable.v2.api

I'm 100% in favor of using the v2 APIs in more places - we desperately need that dog-fooding on them and this is a great way to do that. But if we're going to do this then we need to rip out that sysprop.

Ideally that'd happen before this PR gets merged, just to make sure we don't forget about it and accidentally release something where the bin/solr tools could be effectively bricked on any Solr deploys that use the sysprop. But I'd also be OK with filing a "Blocker" JIRA ticket, or a promise to tackle it soon, or something similar.

Good find! Yeah, I had forgotten about that setting. We could just commit these to main but I do worry about how much pain we set our selves up for in branch_10x.

For various v2 api's that we want to have flexiblity on, can we keep them as "experimental" for a little longer, so we don't have to worry about back compat, with the idea that internal to solr consumers, like the CLI (or maybe internal APIS) wouldn't care that we are changing them over time...

[gerlowskija]

I used Copilot for this, and here is the agent chat: ...

FYI that these links are "private" AFAIK - I get a 404 when trying to click through.

can we keep them as "experimental" for a little longer

I started out writing a reply to this that said essentially: "Sure, why not. Solr offers plenty of feature flags, and there's no clear correlation between those and 'experimental'-ness. Lots of non-experimental features have flags. And there are plenty of other 'experimental' features that don't have a feature flag. No reason to tie these things together."

But in writing that and reading it over I find I've unconvinced myself 😦

The problem here is security. I don't know of any security issues specific to the v2 API, but it's a whole new attack surface. While we're signaling that it's not ready for prime time then I think a subset of users are going to want to be able to disable it. Telling them: "here's this thing that's not quite ready yet, pray it doesn't expose you in some way" is not a great message.

This feels like a much larger discussion and I don't want it to hold up this PR. Would you be OK making this "main"-only for now, and I'll file a JIRA ticket or dev@ thread to figure out a path forward here? The dust may settle on that discussion in a few weeks and we decide we're OK with this hitting branch_10x, but keeping it "main" only for the moment helps you stay unblocked while we figure out the larger question...

[epugh]

I used Copilot for this, and here is the agent chat: ...

FYI that these links are "private" AFAIK - I get a 404 when trying to click through.

can we keep them as "experimental" for a little longer

I started out writing a reply to this that said essentially: "Sure, why not. Solr offers plenty of feature flags, and there's no clear correlation between those and 'experimental'-ness. Lots of non-experimental features have flags. And there are plenty of other 'experimental' features that don't have a feature flag. No reason to tie these things together."

But in writing that and reading it over I find I've unconvinced myself 😦

The problem here is security. I don't know of any security issues specific to the v2 API, but it's a whole new attack surface. While we're signaling that it's not ready for prime time then I think a subset of users are going to want to be able to disable it. Telling them: "here's this thing that's not quite ready yet, pray it doesn't expose you in some way" is not a great message.

This feels like a much larger discussion and I don't want it to hold up this PR. Would you be OK making this "main"-only for now, and I'll file a JIRA ticket or dev@ thread to figure out a path forward here? The dust may settle on that discussion in a few weeks and we decide we're OK with this hitting branch_10x, but keeping it "main" only for the moment helps you stay unblocked while we figure out the larger question...

I love a suggestion that keeps me unblocked ;-). I think main works, and then we can back port later as we decide we want this.

Interesting, I think of these as experimental not because of security but because we aren't sure we have the right flavour of API. Somethign I've noticed lately is that when I build APIs, that then, later, when I use them "for real" I find a lot of issues with them not being "quite right" yet. Not because of security or performacnce, but maybe because of too much paylard returned or not enough filters or what not. So yeah, please do move forward, and I'll merge this to main only.

[gerlowskija]

Found one bug in your code I think; see comments inline.

Also, this PR adds a whole new API (ListClusterNodes), so it probably deserves a changelog entry and some tests that use the generated v2 request.

[gerlowskija]

[0] I may be out of date, but as of one point at least these generated v2 request/response objects didn't throw exception on errors the way everything else in SolrJ does.

Assuming this is still true, users are responsible for inspecting the .responseHeader.status field to make sure that the operation succeeded. Hopefully I'm wrong, but if not you'll need to add that checking here and elsewhere.

[gerlowskija]

[-1] The original code here was grabbing the .cluster.collections bit of the CLUSTERSTATUS response, which includes tons of info:

  "cluster":{
    "live_nodes":["localhost:8983_solr"],
    "collections":{
      "foo":{
        "pullReplicas":"0",
        "configName":"foo",
        "nrtReplicas":1,
        "replicationFactor":1,
        "tlogReplicas":"0",
        "router":{
          "name":"compositeId"
        },
        ....

Are you dropping all that info on purpose in your replacement, or was that a mistake?

[gerlowskija]

[0] IMO the API comparison is a bit clearer if you put the verbatim v1 request here that you're comparing the v2 API to, rather than trying to describe it.

i.e. /admin/collections?action=CLUSTERSTATUS&liveNodes=true&includeAll=false

[epugh]

Found one bug in your code I think; see comments inline.

Also, this PR adds a whole new API (ListClusterNodes), so it probably deserves a changelog entry and some tests that use the generated v2 request.

I am a bit fuzzy on did I ADD a whole new API? Or did I just complete what already existed? Checking

Also, I think it can't be retunring just live nodes if its supposed to return all nodes?