Skip to content

Commit 7ae2631

Browse files
smalyshevsmalyshev
committed
Add security exceptions to the list of fatals and remove test overrides
1 parent 0f4b1b2 commit 7ae2631

File tree

10 files changed

+30
-54
lines changed

10 files changed

+30
-54
lines changed

‎x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java‎

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -315,10 +315,7 @@ public void testIndexPatternErrorMessageComparison_ESQL_SearchDSL() throws Excep
315315
setUser(searchRequest, "metadata1_read2");
316316

317317
// ES|QL query on the same index pattern
318-
var esqlResp = expectThrows(
319-
ResponseException.class,
320-
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2", false)
321-
);
318+
var esqlResp = expectThrows(ResponseException.class, () -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2"));
322319
var srchResp = expectThrows(ResponseException.class, () -> client().performRequest(searchRequest));
323320

324321
for (ResponseException r : List.of(esqlResp, srchResp)) {
@@ -337,8 +334,7 @@ public void testLimitedPrivilege() throws Exception {
337334
ResponseException.class,
338335
() -> runESQLCommand(
339336
"metadata1_read2",
340-
"FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)",
341-
false
337+
"FROM index-user1,index-user2 METADATA _index | STATS sum=sum(value), index=VALUES(_index)"
342338
)
343339
);
344340
assertThat(
@@ -351,7 +347,7 @@ public void testLimitedPrivilege() throws Exception {
351347

352348
resp = expectThrows(
353349
ResponseException.class,
354-
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 METADATA _index | STATS index=VALUES(_index)", false)
350+
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 METADATA _index | STATS index=VALUES(_index)")
355351
);
356352
assertThat(
357353
EntityUtils.toString(resp.getResponse().getEntity()),
@@ -363,7 +359,7 @@ public void testLimitedPrivilege() throws Exception {
363359

364360
resp = expectThrows(
365361
ResponseException.class,
366-
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 | STATS sum=sum(value)", false)
362+
() -> runESQLCommand("metadata1_read2", "FROM index-user1,index-user2 | STATS sum=sum(value)")
367363
);
368364
assertThat(
369365
EntityUtils.toString(resp.getResponse().getEntity()),
@@ -375,7 +371,7 @@ public void testLimitedPrivilege() throws Exception {
375371

376372
resp = expectThrows(
377373
ResponseException.class,
378-
() -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10", false)
374+
() -> runESQLCommand("alias_user1", "FROM first-alias,index-user1 METADATA _index | KEEP _index, org, value | LIMIT 10")
379375
);
380376
assertThat(
381377
EntityUtils.toString(resp.getResponse().getEntity()),
@@ -389,8 +385,7 @@ public void testLimitedPrivilege() throws Exception {
389385
ResponseException.class,
390386
() -> runESQLCommand(
391387
"alias_user2",
392-
"from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)",
393-
false
388+
"from second-alias,index-user2 METADATA _index | stats sum=sum(value), index=VALUES(_index)"
394389
)
395390
);
396391
assertThat(
@@ -864,10 +859,6 @@ public void testDataStream() throws IOException {
864859
}
865860

866861
protected Response runESQLCommand(String user, String command) throws IOException {
867-
return runESQLCommand(user, command, null);
868-
}
869-
870-
protected Response runESQLCommand(String user, String command, Boolean allowPartialResults) throws IOException {
871862
if (command.toLowerCase(Locale.ROOT).contains("limit") == false) {
872863
// add a (high) limit to avoid warnings on default limit
873864
command += " | limit 10000000";
@@ -881,9 +872,6 @@ protected Response runESQLCommand(String user, String command, Boolean allowPart
881872
request.setJsonEntity(Strings.toString(json));
882873
setUser(request, user);
883874
request.addParameter("error_trace", "true");
884-
if (allowPartialResults != null) {
885-
request.addParameter("allow_partial_results", Boolean.toString(allowPartialResults));
886-
}
887875
return client().performRequest(request);
888876
}
889877

‎x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/RestEsqlIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public void testInvalidPragma() throws IOException {
111111
request.setJsonEntity("{\"f\":" + i + "}");
112112
assertOK(client().performRequest(request));
113113
}
114-
RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f").allowPartialResults(false);
114+
RequestObjectBuilder builder = requestObjectBuilder().query("from test-index | limit 1 | keep f");
115115
builder.pragmas(Settings.builder().put("data_partitioning", "invalid-option").build());
116116
ResponseException re = expectThrows(ResponseException.class, () -> runEsqlSync(builder));
117117
assertThat(EntityUtils.toString(re.getResponse().getEntity()), containsString("No enum constant"));

‎x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/EsqlRestValidationTestCase.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ private Request createRequest(String indexName) throws IOException {
129129
final var request = new Request("POST", "/_query");
130130
request.addParameter("error_trace", "true");
131131
request.addParameter("pretty", "true");
132-
request.addParameter("allow_partial_results", Boolean.toString(false));
133132
request.setJsonEntity(
134133
Strings.toString(JsonXContent.contentBuilder().startObject().field("query", "from " + indexName).endObject())
135134
);

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractCrossClusterTestCase.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins(String clusterAlias) {
7777
return plugins;
7878
}
7979

80-
@Override
81-
protected Settings nodeSettings() {
82-
return Settings.builder().put(super.nodeSettings()).put(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.getKey(), false).build();
83-
}
84-
8580
public static class InternalExchangePlugin extends Plugin {
8681
@Override
8782
public List<Setting<?>> getSettings() {

‎x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,6 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
139139
return CollectionUtils.appendToCopy(super.nodePlugins(), EsqlPlugin.class);
140140
}
141141

142-
@Override
143-
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
144-
return Settings.builder()
145-
.put(super.nodeSettings(nodeOrdinal, otherSettings))
146-
.put(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.getKey(), false)
147-
.build();
148-
}
149-
150142
protected void setRequestCircuitBreakerLimit(ByteSizeValue limit) {
151143
if (limit != null) {
152144
assertAcked(

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.plugin;
99

10-
import org.elasticsearch.ExceptionsHelper;
1110
import org.elasticsearch.action.ActionListener;
1211
import org.elasticsearch.action.ActionListenerResponseHandler;
1312
import org.elasticsearch.action.OriginalIndices;
@@ -17,7 +16,6 @@
1716
import org.elasticsearch.compute.operator.exchange.ExchangeSourceHandler;
1817
import org.elasticsearch.core.Releasable;
1918
import org.elasticsearch.core.TimeValue;
20-
import org.elasticsearch.index.IndexNotFoundException;
2119
import org.elasticsearch.tasks.CancellableTask;
2220
import org.elasticsearch.tasks.Task;
2321
import org.elasticsearch.tasks.TaskCancelledException;
@@ -90,18 +88,12 @@ void startComputeOnRemoteCluster(
9088
if (receivedResults == false && EsqlCCSUtils.shouldIgnoreRuntimeError(executionInfo, clusterAlias, e)) {
9189
EsqlCCSUtils.markClusterWithFinalStateAndNoShards(executionInfo, clusterAlias, EsqlExecutionInfo.Cluster.Status.SKIPPED, e);
9290
l.onResponse(DriverCompletionInfo.EMPTY);
93-
} else if (configuration.allowPartialResults()
94-
&& (ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException) == false) {
95-
EsqlCCSUtils.markClusterWithFinalStateAndNoShards(
96-
executionInfo,
97-
clusterAlias,
98-
EsqlExecutionInfo.Cluster.Status.PARTIAL,
99-
e
100-
);
101-
l.onResponse(DriverCompletionInfo.EMPTY);
102-
} else {
103-
l.onFailure(e);
104-
}
91+
} else if (configuration.allowPartialResults() && EsqlCCSUtils.canAllowPartial(e)) {
92+
EsqlCCSUtils.markClusterWithFinalStateAndNoShards(executionInfo, clusterAlias, EsqlExecutionInfo.Cluster.Status.PARTIAL, e);
93+
l.onResponse(DriverCompletionInfo.EMPTY);
94+
} else {
95+
l.onFailure(e);
96+
}
10597
});
10698
ExchangeService.openExchange(
10799
transportService,

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package org.elasticsearch.xpack.esql.plugin;
99

10-
import org.elasticsearch.ExceptionsHelper;
1110
import org.elasticsearch.action.ActionListener;
1211
import org.elasticsearch.action.OriginalIndices;
1312
import org.elasticsearch.action.search.SearchRequest;
@@ -31,7 +30,6 @@
3130
import org.elasticsearch.core.Releasable;
3231
import org.elasticsearch.core.Releasables;
3332
import org.elasticsearch.core.Tuple;
34-
import org.elasticsearch.index.IndexNotFoundException;
3533
import org.elasticsearch.index.query.SearchExecutionContext;
3634
import org.elasticsearch.logging.LogManager;
3735
import org.elasticsearch.logging.Logger;
@@ -63,6 +61,7 @@
6361
import org.elasticsearch.xpack.esql.planner.LocalExecutionPlanner;
6462
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
6563
import org.elasticsearch.xpack.esql.session.Configuration;
64+
import org.elasticsearch.xpack.esql.session.EsqlCCSUtils;
6665
import org.elasticsearch.xpack.esql.session.Result;
6766

6867
import java.util.ArrayList;
@@ -433,8 +432,7 @@ public void executePlan(
433432
);
434433
dataNodesListener.onResponse(r.getCompletionInfo());
435434
}, e -> {
436-
if (configuration.allowPartialResults()
437-
&& (ExceptionsHelper.unwrapCause(e) instanceof IndexNotFoundException) == false) {
435+
if (configuration.allowPartialResults() && EsqlCCSUtils.canAllowPartial(e)) {
438436
execInfo.swapCluster(
439437
LOCAL_CLUSTER,
440438
(k, v) -> new EsqlExecutionInfo.Cluster.Builder(v).setStatus(

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
package org.elasticsearch.xpack.esql.session;
99

10+
import org.elasticsearch.ElasticsearchException;
11+
import org.elasticsearch.ElasticsearchSecurityException;
1012
import org.elasticsearch.ExceptionsHelper;
1113
import org.elasticsearch.action.ActionListener;
1214
import org.elasticsearch.action.OriginalIndices;
@@ -17,6 +19,7 @@
1719
import org.elasticsearch.common.util.set.Sets;
1820
import org.elasticsearch.compute.operator.DriverCompletionInfo;
1921
import org.elasticsearch.core.Nullable;
22+
import org.elasticsearch.index.IndexNotFoundException;
2023
import org.elasticsearch.index.query.QueryBuilder;
2124
import org.elasticsearch.indices.IndicesExpressionGrouper;
2225
import org.elasticsearch.license.XPackLicenseState;
@@ -368,4 +371,16 @@ public static boolean shouldIgnoreRuntimeError(EsqlExecutionInfo executionInfo,
368371

369372
return ExceptionsHelper.isRemoteUnavailableException(e);
370373
}
374+
375+
/**
376+
* Check whether this exception can be tolerated when partial results are on, or should be treated as fatal.
377+
* @return true if the exception can be tolerated, false if it should be treated as fatal.
378+
*/
379+
public static boolean canAllowPartial(Exception e) {
380+
Throwable unwrapped = ExceptionsHelper.unwrapCause(e);
381+
if (unwrapped instanceof IndexNotFoundException || unwrapped instanceof ElasticsearchSecurityException) {
382+
return false;
383+
}
384+
return true;
385+
}
371386
}

‎x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ public class RemoteClusterSecurityEsqlIT extends AbstractRemoteClusterSecurityTe
7878
.apply(commonClusterConfig)
7979
.setting("remote_cluster.port", "0")
8080
.setting("xpack.ml.enabled", "false")
81-
.setting("esql.query.allow_partial_results", "false")
8281
.setting("xpack.security.remote_cluster_server.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get()))
8382
.setting("xpack.security.remote_cluster_server.ssl.key", "remote-cluster.key")
8483
.setting("xpack.security.remote_cluster_server.ssl.certificate", "remote-cluster.crt")
@@ -98,7 +97,6 @@ public class RemoteClusterSecurityEsqlIT extends AbstractRemoteClusterSecurityTe
9897
.module("ingest-common")
9998
.apply(commonClusterConfig)
10099
.setting("xpack.ml.enabled", "false")
101-
.setting("esql.query.allow_partial_results", "false")
102100
.setting("xpack.security.remote_cluster_client.ssl.enabled", () -> String.valueOf(SSL_ENABLED_REF.get()))
103101
.setting("xpack.security.remote_cluster_client.ssl.certificate_authorities", "remote-cluster-ca.crt")
104102
.setting("xpack.security.authc.token.enabled", "true")

‎x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2771,7 +2771,6 @@ Request toEsqlRequest() throws IOException {
27712771
json.endObject();
27722772
var request = new Request("POST", "_query");
27732773
request.setJsonEntity(Strings.toString(json));
2774-
request.addParameter("allow_partial_results", Boolean.toString(false));
27752774
return request;
27762775
}
27772776
}

0 commit comments

Comments
 (0)