Skip to content

Fix v2 API 404 when a /config request is proxied to another node - #5178

Open
epugh wants to merge 3 commits into
apache:mainfrom
epugh:fix-v2-proxy-collection-alias
Open

epugh wants to merge 3 commits into
apache:mainfrom
epugh:fix-v2-proxy-collection-alias

Conversation

@epugh

@epugh epugh commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Solr forwards a v2 request to the node that actually hosts the target collection using the short /c/{collection}/... path alias (see HttpSolrCall#sendRemoteProxy), but Constants.INDEX_PATH_PREFIX's JAX-RS @Path regex only recognized cores or collections, not c. Any JAX-RS resource built on this prefix therefore 404'd with Cannot find API for the path whenever a request had to be proxied cross-node.
  • Widened the regex to accept c as an alias for collections.
  • This was discovered while migrating GET /config/requestHandler to JAX-RS (SOLR-16396): PackageToolTest uses a 2-node cluster and intermittently hit this exact 404 whenever its HTTP client landed on the node that didn't host the test collection's replica. Confirmed the bug is general (not specific to that migration) by reproducing it against an already-merged, unrelated endpoint (GET /config/overlay).

Test plan

  • Added GetConfigSolrCloudTest.testGetOverlayFromCollectionViaNonHostingNode, which bumps the test cluster to 2 nodes and deliberately sends a request to the node that does not host the collection's replica, forcing an internal proxy. Verified it fails with Cannot find API for the path: /config/overlay before the fix, and passes after.
  • Full GetConfigSolrCloudTest suite passes.
  • org.apache.solr.handler.admin.api.*, org.apache.solr.handler.admin.*, org.apache.solr.api.*, org.apache.solr.jersey.* (436 tests) pass.
  • org.apache.solr.cloud.api.collections.*, org.apache.solr.handler.*V2*, org.apache.solr.servlet.* pass.
  • spotlessApply — no formatting changes needed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5

Solr forwards a v2 request to the node that actually hosts the target
collection using the short "/c/{collection}/..." path alias (see
HttpSolrCall#sendRemoteProxy), but Constants.INDEX_PATH_PREFIX's JAX-RS
@path regex only recognized "cores" or "collections", not "c". Any
JAX-RS resource built on that prefix therefore 404'd with "Cannot find
API for the path" whenever a request needed to be proxied cross-node.

Reproduced with a new 2-node test in GetConfigSolrCloudTest that
deliberately sends a GET /config/overlay request to the node that does
NOT host the collection's only replica, then fixed by accepting "c" as
an alias for "collections" in the path regex.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5
@epugh

epugh commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

@jaykay12 this is the actual issue you were bumping into with package tool test!!!

@epugh

epugh commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

@gerlowskija would love your review. Also should we just drop the /c/ thing???

@epugh

epugh commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

This was discovered while reviewing/debugging #5168 (SOLR-16396: migrate `GET /config/requestHandler` to JAX-RS) — its `PackageToolTest` was intermittently failing with `Cannot find API for the path: /config/requestHandlers` whenever the test's 2-node cluster routed the HTTP request to the node that didn't host the test collection's replica. Traced it down to this proxy-routing gap, confirmed it's general (not specific to that migration) by reproducing it against the already-merged `GET /config/overlay` endpoint here, and split the fix out into its own PR since it's independent of that migration.

@epugh

epugh commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

/no-changelog

CI builds with -Werror, and errorprone flags comparing JettySolrRunner
instances with != instead of .equals().

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5
@epugh
epugh requested a review from gerlowskija October 17, 2026 15:25
Comment thread solr/core/src/test/org/apache/solr/handler/admin/api/GetConfigSolrCloudTest.java Outdated
Comment on lines +28 to +32
// "c" is accepted alongside "collections" because Solr internally forwards a v2 request to the
// node actually hosting a collection using the short "/c/{collection}/..." alias (see
// HttpSolrCall#sendRemoteProxy and V2HttpCall's "c"/"collections" handling); without it, any
// JAX-RS resource built on this prefix 404s with "Cannot find API for the path" whenever the
// request has to be proxied to another node.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder do we really need this justification. It doesn't look strange for 'c' to be here (to me); I'm not going to wonder what would break if one of these items was missing.

The method name already says what the test does; per dsmiley's review
comment, the javadoc was redundant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RzzUgiebQ8ZhhgE2E7ZZU5
@sigram

sigram commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Looks good to me. Claude Opus 5.5 has this to say:

Real but minor effects:

  1. The alias covers every resource using the shared prefix, not just /config. That is Select, GetSchema, UpdateSchema, Config and Tasks, and each becomes reachable directly at /api/c/{coll}/.... I think that's the intended behaviour, but the PR title only mentions /config, so it's worth saying in the PR description and changelog.
  2. The test class now starts a 2-node cluster for every test, which is slightly slower. That's acceptable.

Nits:

  • The comment at solr/webapp/web/js/angular/services.js:266 still says (cores|collections).

Result: authorization, proxy dispatch and generated clients are unaffected, and the c alias now applies to all five resources using the shared prefix. Remaining items are minor: an incomplete comment, and the PR title understating the scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants