Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
epugh merged 3 commits into from
Oct 20, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc37b68
Fix v2 API 404 when a /config request is proxied to another node
epugh Oct 17, 2026
5b16859
Fix ErrorProne ReferenceEquality failure in the new regression test
epugh Oct 17, 2026
3e6817c
Drop over-explaining javadoc on the regression test
epugh Oct 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion solr/api/src/java/org/apache/solr/client/api/util/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,18 @@ private Constants() {

public static final String INDEX_TYPE_PATH_PARAMETER = "indexType";
public static final String INDEX_NAME_PATH_PARAMETER = "indexName";

// "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.
Comment on lines +28 to +32

@dsmiley dsmiley Oct 17, 2026

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.

@dsmiley dsmiley Oct 20, 2026

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 suppose silence means implied disagreement or lack of interest.
I find an increasing part of my code reviewing job in the age of AI coding development is to get Claude to shut up when it's saying too much :-)

@epugh epugh Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

humm.... I think I missed your comment.... I will say, I 've though about going through various modules and seeing if we could put our comments on a diet... kind of like when we used to go through looking for stylistic warnnings from intellji!

@dsmiley dsmiley Oct 21, 2026

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.

It's not a codebase-wide issue (yet), it's on many AI-driven PRs like this one.

public static final String INDEX_PATH_PREFIX =
"/{" + INDEX_TYPE_PATH_PARAMETER + ":cores|collections}/{" + INDEX_NAME_PATH_PARAMETER + "}";
"/{"
+ INDEX_TYPE_PATH_PARAMETER
+ ":cores|collections|c}/{"
+ INDEX_NAME_PATH_PARAMETER
+ "}";

public static final String CORE_NAME_PATH_PARAMETER = "coreName";

Expand Down
28 changes: 27 additions & 1 deletion solr/core/src/test/org/apache/solr/handler/admin/api/GetConfigSolrCloudTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
import static org.apache.solr.core.CoreContainer.ALLOW_PATHS_SYSPROP;

import org.apache.solr.client.api.model.IndexType;
import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.ConfigApi;
import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.embedded.JettySolrRunner;
import org.apache.solr.util.ExternalPaths;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -34,7 +37,9 @@ public class GetConfigSolrCloudTest extends SolrCloudTestCase {
@BeforeClass
public static void setupCluster() throws Exception {
System.setProperty(ALLOW_PATHS_SYSPROP, ExternalPaths.SERVER_HOME.toAbsolutePath().toString());
configureCluster(1).addConfig("conf", configset("cloud-minimal")).configure();
// 2 nodes so that a request can land on a node that does *not* host the collection's only
// replica, forcing Solr to internally proxy the request to the node that does.
configureCluster(2).addConfig("conf", configset("cloud-minimal")).configure();
CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf", 1, 1)
.process(cluster.getSolrClient());
}
Expand Down Expand Up @@ -77,6 +82,27 @@ public void testGetZnodeVersionFromCollection() throws Exception {
assertTrue(response.znodeVersion.params >= -1);
}

@Test
public void testGetOverlayFromCollectionViaNonHostingNode() throws Exception {
Replica replica = getCollectionState(COLLECTION_NAME).replicaStream().findFirst().orElseThrow();
JettySolrRunner hostingJetty = cluster.getReplicaJetty(replica);
JettySolrRunner otherJetty =
cluster.getJettySolrRunners().stream()
.filter(jetty -> !jetty.equals(hostingJetty))
.findFirst()
.orElseThrow();

try (var client = new HttpJettySolrClient.Builder(otherJetty.getBaseUrl().toString()).build()) {
var request = new ConfigApi.GetOverlay(IndexType.COLLECTION, COLLECTION_NAME);
var response = request.process(client);

assertNotNull(response);
assertNull(response.error);
assertNotNull(response.overlay);
assertTrue(response.overlay.containsKey("znodeVersion"));
}
}

@Test
public void testGetConfigComponentFromCollection() throws Exception {
for (String component : GetConfig.MIGRATED_CONFIG_COMPONENTS) {
Expand Down
Loading