Allow ESQL extra verifiers to access settings (#129954)

This commit is contained in:
Mark Vieira 2025-06-25 16:43:56 -07:00 committed by GitHub
parent bf2a283315
commit e6cf2234fc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 49 additions and 175 deletions

View file

@ -1,29 +0,0 @@
apply plugin: 'elasticsearch.internal-es-plugin'
apply plugin: 'elasticsearch.internal-java-rest-test'
apply plugin: org.elasticsearch.gradle.internal.precommit.CheckstylePrecommitPlugin
apply plugin: org.elasticsearch.gradle.internal.precommit.ForbiddenApisPrecommitPlugin
apply plugin: org.elasticsearch.gradle.internal.precommit.ForbiddenPatternsPrecommitPlugin
apply plugin: org.elasticsearch.gradle.internal.precommit.FilePermissionsPrecommitPlugin
apply plugin: org.elasticsearch.gradle.internal.precommit.LoggerUsagePrecommitPlugin
apply plugin: org.elasticsearch.gradle.internal.precommit.TestingConventionsPrecommitPlugin
esplugin {
name = 'extra-checkers'
description = 'An example plugin disallowing CATEGORIZE'
classname ='org.elasticsearch.xpack.esql.qa.extra.ExtraCheckersPlugin'
extendedPlugins = ['x-pack-esql']
}
dependencies {
compileOnly project(':x-pack:plugin:esql')
compileOnly project(':x-pack:plugin:esql-core')
clusterPlugins project(':x-pack:plugin:esql:qa:server:extra-checkers')
}
tasks.named('javaRestTest') {
usesDefaultDistribution("to be triaged")
maxParallelForks = 1
jvmArgs('--add-opens=java.base/java.nio=ALL-UNNAMED')
}

View file

@ -1,80 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.xpack.esql.qa.extra;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import org.apache.http.util.EntityUtils;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.test.TestClustersThreadFilter;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
import org.elasticsearch.test.rest.ESRestTestCase;
import org.junit.ClassRule;
import java.io.IOException;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
@ThreadLeakFilters(filters = TestClustersThreadFilter.class)
public class ExtraCheckersIT extends ESRestTestCase {
@ClassRule
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
.distribution(DistributionType.DEFAULT)
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
.shared(true)
.plugin("extra-checkers")
.build();
public void testWithCategorize() {
ResponseException e = expectThrows(ResponseException.class, () -> runEsql("""
{
"query": "ROW message=\\"foo bar\\" | STATS COUNT(*) BY CATEGORIZE(message) | LIMIT 1"
}"""));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(400));
assertThat(e.getMessage(), containsString("line 1:43: CATEGORIZE is unsupported"));
}
public void testWithoutCategorize() throws IOException {
String result = runEsql("""
{
"query": "ROW message=\\"foo bar\\" | STATS COUNT(*) | LIMIT 1"
}""");
assertThat(result, containsString("""
"columns" : [
{
"name" : "COUNT(*)",
"type" : "long"
}
],
"values" : [
[
1
]
]
"""));
}
private String runEsql(String json) throws IOException {
Request request = new Request("POST", "/_query");
request.setJsonEntity(json);
request.addParameter("error_trace", "");
request.addParameter("pretty", "");
Response response = client().performRequest(request);
return EntityUtils.toString(response.getEntity());
}
@Override
protected String getTestRestCluster() {
return cluster.getHttpAddresses();
}
}

View file

@ -1,31 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.xpack.esql.qa.extra;
import org.elasticsearch.xpack.esql.analysis.Verifier;
import org.elasticsearch.xpack.esql.common.Failure;
import org.elasticsearch.xpack.esql.common.Failures;
import org.elasticsearch.xpack.esql.expression.function.grouping.Categorize;
import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
import java.util.List;
import java.util.function.BiConsumer;
public class DisallowCategorize implements Verifier.ExtraCheckers {
@Override
public List<BiConsumer<LogicalPlan, Failures>> extra() {
return List.of(DisallowCategorize::disallowCategorize);
}
private static void disallowCategorize(LogicalPlan plan, Failures failures) {
if (plan instanceof Aggregate) {
plan.forEachExpression(Categorize.class, cat -> failures.add(new Failure(cat, "CATEGORIZE is unsupported")));
}
}
}

View file

@ -1,16 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.xpack.esql.qa.extra;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.xpack.esql.analysis.Verifier;
/**
* Marker plugin to enable {@link Verifier.ExtraCheckers}.
*/
public class ExtraCheckersPlugin extends Plugin {}

View file

@ -1,8 +0,0 @@
#
# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
# or more contributor license agreements. Licensed under the Elastic License
# 2.0; you may not use this file except in compliance with the Elastic License
# 2.0.
#
org.elasticsearch.xpack.esql.qa.extra.DisallowCategorize

View file

@ -87,6 +87,10 @@ public class EsqlActionBreakerIT extends EsqlActionIT {
}
public static class EsqlTestPluginWithMockBlockFactory extends EsqlPlugin {
public EsqlTestPluginWithMockBlockFactory(Settings settings) {
super(settings);
}
@Override
protected BlockFactoryProvider blockFactoryProvider(
CircuitBreaker breaker,

View file

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.esql.action;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.internal.XPackLicenseStatus;
@ -19,6 +20,10 @@ import static org.elasticsearch.test.ESTestCase.randomFrom;
* that require an Enteprise (or Trial) license.
*/
public class EsqlPluginWithEnterpriseOrTrialLicense extends EsqlPlugin {
public EsqlPluginWithEnterpriseOrTrialLicense(Settings settings) {
super(settings);
}
protected XPackLicenseState getLicenseState() {
License.OperationMode operationMode = randomFrom(License.OperationMode.ENTERPRISE, License.OperationMode.TRIAL);
return new XPackLicenseState(() -> System.currentTimeMillis(), new XPackLicenseStatus(operationMode, true, "Test license expired"));

View file

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.esql.action;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.internal.XPackLicenseStatus;
@ -22,6 +23,10 @@ import static org.elasticsearch.test.ESTestCase.randomFrom;
* - an expired enterprise or trial license
*/
public class EsqlPluginWithNonEnterpriseOrExpiredLicense extends EsqlPlugin {
public EsqlPluginWithNonEnterpriseOrExpiredLicense(Settings settings) {
super(settings);
}
protected XPackLicenseState getLicenseState() {
License.OperationMode operationMode;
boolean active;

View file

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.esql.spatial;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.internal.XPackLicenseStatus;
@ -49,6 +50,10 @@ public abstract class SpatialNoLicenseTestCase extends ESIntegTestCase {
* This is used to test the behavior of spatial functions when no valid license is present.
*/
public static class TestEsqlPlugin extends EsqlPlugin {
public TestEsqlPlugin(Settings settings) {
super(settings);
}
protected XPackLicenseState getLicenseState() {
return SpatialNoLicenseTestCase.getLicenseState();
}

View file

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.esql.analysis;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.xpack.esql.LicenseAware;
import org.elasticsearch.xpack.esql.capabilities.PostAnalysisPlanVerificationAware;
@ -60,7 +61,7 @@ public class Verifier {
* Build a list of checks to perform on the plan. Each one is called once per
* {@link LogicalPlan} node in the plan.
*/
List<BiConsumer<LogicalPlan, Failures>> extra();
List<BiConsumer<LogicalPlan, Failures>> extra(Settings settings);
}
/**
@ -69,15 +70,17 @@ public class Verifier {
private final List<ExtraCheckers> extraCheckers;
private final Metrics metrics;
private final XPackLicenseState licenseState;
private final Settings settings;
public Verifier(Metrics metrics, XPackLicenseState licenseState) {
this(metrics, licenseState, Collections.emptyList());
this(metrics, licenseState, Collections.emptyList(), Settings.EMPTY);
}
public Verifier(Metrics metrics, XPackLicenseState licenseState, List<ExtraCheckers> extraCheckers) {
public Verifier(Metrics metrics, XPackLicenseState licenseState, List<ExtraCheckers> extraCheckers, Settings settings) {
this.metrics = metrics;
this.licenseState = licenseState;
this.extraCheckers = extraCheckers;
this.settings = settings;
}
/**
@ -102,7 +105,7 @@ public class Verifier {
// collect plan checkers
var planCheckers = planCheckers(plan);
for (ExtraCheckers e : extraCheckers) {
planCheckers.addAll(e.extra());
planCheckers.addAll(e.extra(settings));
}
// Concrete verifications

View file

@ -8,6 +8,7 @@
package org.elasticsearch.xpack.esql.execution;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.indices.IndicesExpressionGrouper;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.telemetry.metric.MeterRegistry;
@ -52,14 +53,15 @@ public class PlanExecutor {
MeterRegistry meterRegistry,
XPackLicenseState licenseState,
EsqlQueryLog queryLog,
List<Verifier.ExtraCheckers> extraCheckers
List<Verifier.ExtraCheckers> extraCheckers,
Settings settings
) {
this.indexResolver = indexResolver;
this.preAnalyzer = new PreAnalyzer();
this.functionRegistry = new EsqlFunctionRegistry();
this.mapper = new Mapper();
this.metrics = new Metrics(functionRegistry);
this.verifier = new Verifier(metrics, licenseState, extraCheckers);
this.verifier = new Verifier(metrics, licenseState, extraCheckers, settings);
this.planTelemetryManager = new PlanTelemetryManager(meterRegistry);
this.queryLog = queryLog;
}

View file

@ -190,6 +190,11 @@ public class EsqlPlugin extends Plugin implements ActionPlugin, ExtensiblePlugin
);
private final List<Verifier.ExtraCheckers> extraCheckers = new ArrayList<>();
private final Settings settings;
public EsqlPlugin(Settings settings) {
this.settings = settings;
}
@Override
public Collection<?> createComponents(PluginServices services) {
@ -209,7 +214,8 @@ public class EsqlPlugin extends Plugin implements ActionPlugin, ExtensiblePlugin
services.telemetryProvider().getMeterRegistry(),
getLicenseState(),
new EsqlQueryLog(services.clusterService().getClusterSettings(), services.slowLogFieldProvider()),
extraCheckers
extraCheckers,
settings
),
new ExchangeService(
services.clusterService().getSettings(),

View file

@ -8,6 +8,7 @@
package org.elasticsearch.xpack.esql.action;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.compute.operator.Operator;
import org.elasticsearch.compute.operator.topn.TopNOperatorStatus;
import org.elasticsearch.test.ESTestCase;
@ -18,7 +19,7 @@ import static org.hamcrest.Matchers.equalTo;
public class NamedWriteablesTests extends ESTestCase {
public void testTopNStatus() throws Exception {
try (EsqlPlugin plugin = new EsqlPlugin()) {
try (EsqlPlugin plugin = new EsqlPlugin(Settings.EMPTY)) {
NamedWriteableRegistry registry = new NamedWriteableRegistry(plugin.getNamedWriteables());
TopNOperatorStatus origin = new TopNOperatorStatus(
randomNonNegativeInt(),

View file

@ -57,7 +57,7 @@ public class ClusterRequestTests extends AbstractWireSerializingTestCase<Cluster
protected NamedWriteableRegistry getNamedWriteableRegistry() {
List<NamedWriteableRegistry.Entry> writeables = new ArrayList<>();
writeables.addAll(new SearchModule(Settings.EMPTY, List.of()).getNamedWriteables());
writeables.addAll(new EsqlPlugin().getNamedWriteables());
writeables.addAll(new EsqlPlugin(Settings.EMPTY).getNamedWriteables());
return new NamedWriteableRegistry(writeables);
}

View file

@ -60,7 +60,7 @@ public class DataNodeRequestSerializationTests extends AbstractWireSerializingTe
protected NamedWriteableRegistry getNamedWriteableRegistry() {
List<NamedWriteableRegistry.Entry> writeables = new ArrayList<>();
writeables.addAll(new SearchModule(Settings.EMPTY, List.of()).getNamedWriteables());
writeables.addAll(new EsqlPlugin().getNamedWriteables());
writeables.addAll(new EsqlPlugin(Settings.EMPTY).getNamedWriteables());
return new NamedWriteableRegistry(writeables);
}

View file

@ -150,7 +150,14 @@ public class PlanExecutorMetricsTests extends ESTestCase {
return null;
}).when(esqlClient).execute(eq(EsqlResolveFieldsAction.TYPE), any(), any());
var planExecutor = new PlanExecutor(indexResolver, MeterRegistry.NOOP, new XPackLicenseState(() -> 0L), mockQueryLog(), List.of());
var planExecutor = new PlanExecutor(
indexResolver,
MeterRegistry.NOOP,
new XPackLicenseState(() -> 0L),
mockQueryLog(),
List.of(),
Settings.EMPTY
);
var enrichResolver = mockEnrichResolver();
var request = new EsqlQueryRequest();