You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by st...@apache.org on 2023/09/06 23:08:44 UTC
[solr] branch branch_9x updated: SOLR-16955 Tracing v2 apis breaks SecurityConfHandler (#1884)
This is an automated email from the ASF dual-hosted git repository.
stillalex pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 4bb2ed39447 SOLR-16955 Tracing v2 apis breaks SecurityConfHandler (#1884)
4bb2ed39447 is described below
commit 4bb2ed39447499ba14b767c5a8066f381f36f96d
Author: Alex D <st...@apache.org>
AuthorDate: Wed Sep 6 15:18:03 2023 -0700
SOLR-16955 Tracing v2 apis breaks SecurityConfHandler (#1884)
---
solr/CHANGES.txt | 2 ++
solr/core/src/java/org/apache/solr/api/V2HttpCall.java | 18 +-----------------
.../apache/solr/handler/admin/SecurityConfHandler.java | 8 ++++++++
.../org/apache/solr/servlet/SolrDispatchFilter.java | 9 ++++++++-
.../java/org/apache/solr/util/tracing/TraceUtils.java | 8 ++++++++
5 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 9ad971f631d..9bb48def458 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -95,6 +95,8 @@ Bug Fixes
* SOLR-16958: Fix spurious warning about LATEST luceneMatchVersion (Colvin Cowie)
+* SOLR-16955: Tracing v2 apis breaks SecurityConfHandler (Alex Deparvu, David Smiley)
+
Dependency Upgrades
---------------------
diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
index 46c66e403df..2f17bb0571b 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -45,7 +45,6 @@ import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.util.CommandOperation;
import org.apache.solr.common.util.JsonSchemaValidator;
import org.apache.solr.common.util.PathTrie;
import org.apache.solr.common.util.SuppressForbidden;
@@ -505,22 +504,7 @@ public class V2HttpCall extends HttpSolrCall {
// Get the templatize-ed path, ex: "/c/{collection}"
final String path = computeEndpointPath();
-
- String verb = null;
- // if this api has commands ...
- final Map<String, JsonSchemaValidator> validators = getValidators(); // should be cached
- if (validators != null && validators.isEmpty() == false && solrReq != null) {
- boolean validateInput = true; // because getCommands caches it; and we want it validated later
- // does this request have one command?
- List<CommandOperation> cmds = solrReq.getCommands(validateInput);
- if (cmds.size() == 1) {
- verb = cmds.get(0).name;
- }
- }
- if (verb == null) {
- verb = req.getMethod().toLowerCase(Locale.ROOT);
- }
-
+ final String verb = req.getMethod().toLowerCase(Locale.ROOT);
span.setOperationName(verb + ":" + path);
}
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
index 9142449cba6..95e1d10f351 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
@@ -28,6 +28,7 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.stream.Collectors;
import org.apache.solr.api.AnnotatedApi;
import org.apache.solr.api.Api;
import org.apache.solr.api.ApiBag;
@@ -52,6 +53,7 @@ import org.apache.solr.security.AuthorizationContext;
import org.apache.solr.security.AuthorizationPlugin;
import org.apache.solr.security.ConfigEditablePlugin;
import org.apache.solr.security.PermissionNameProvider;
+import org.apache.solr.util.tracing.TraceUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -147,6 +149,7 @@ public abstract class SecurityConfHandler extends RequestHandlerBase
if (persistConf(securityConfig)) {
securityConfEdited();
+ updateTraceOps(req, configEditablePlugin.getClass().getSimpleName(), commandsCopy);
return;
}
}
@@ -156,6 +159,11 @@ public abstract class SecurityConfHandler extends RequestHandlerBase
SERVER_ERROR, "Failed to persist security config after 3 attempts. Giving up");
}
+ private void updateTraceOps(SolrQueryRequest req, String clazz, List<CommandOperation> commands) {
+ TraceUtils.setOperations(
+ req, clazz, commands.stream().map(c -> c.name).collect(Collectors.toUnmodifiableList()));
+ }
+
/** Hook where you can do stuff after a config has been edited. Defaults to NOP */
protected void securityConfEdited() {}
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
index cfd9836c4ed..f1b1513ce51 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -246,7 +246,14 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder {
if (log.isDebugEnabled()) {
log.debug("User principal: {}", request.getUserPrincipal());
}
- getSpan(request).setTag(Tags.DB_USER, String.valueOf(request.getUserPrincipal()));
+
+ final String principalName;
+ if (request.getUserPrincipal() != null) {
+ principalName = request.getUserPrincipal().getName();
+ } else {
+ principalName = null;
+ }
+ getSpan(request).setTag(Tags.DB_USER, String.valueOf(principalName));
}
HttpSolrCall call = getHttpSolrCall(request, response, retry);
diff --git a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
index c7d2f0f0fd9..069da747a34 100644
--- a/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
+++ b/solr/core/src/java/org/apache/solr/util/tracing/TraceUtils.java
@@ -19,6 +19,7 @@ package org.apache.solr.util.tracing;
import io.opentracing.Span;
import io.opentracing.noop.NoopSpan;
import io.opentracing.tag.Tags;
+import java.util.List;
import java.util.function.Consumer;
import org.apache.solr.request.SolrQueryRequest;
@@ -36,4 +37,11 @@ public class TraceUtils {
consumer.accept(span);
}
}
+
+ public static void setOperations(SolrQueryRequest req, String clazz, List<String> ops) {
+ if (!ops.isEmpty()) {
+ req.getSpan().setTag("ops", String.join(",", ops));
+ req.getSpan().setTag("class", clazz);
+ }
+ }
}