Skip to content

Commit 01c0d41

Browse files
craigtavernercraigtaverner
committed
Initial work on view service configuration
* Max nested view depth * Max number of view definitions * Max individual view query length
1 parent 6fd3357 commit 01c0d41

File tree

6 files changed

+122
-18
lines changed

6 files changed

+122
-18
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,11 @@ public Collection<?> createComponents(PluginServices services) {
233233
BigArrays bigArrays = services.indicesService().getBigArrays().withCircuitBreaking();
234234
var blockFactoryProvider = blockFactoryProvider(circuitBreaker, bigArrays, maxPrimitiveArrayBlockSize);
235235
EsqlFunctionRegistry functionRegistry = new EsqlFunctionRegistry();
236-
ViewService viewService = new ClusterViewService(functionRegistry, services.clusterService());
236+
ViewService viewService = new ClusterViewService(
237+
functionRegistry,
238+
services.clusterService(),
239+
ViewService.ViewServiceConfig.fromSettings(settings)
240+
);
237241
setupSharedSecrets();
238242
List<BiConsumer<LogicalPlan, Failures>> extraCheckers = extraCheckerProviders.stream()
239243
.flatMap(p -> p.checkers(services.projectResolver(), services.clusterService()).stream())

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
public class ClusterViewService extends ViewService {
2525
private final ClusterService clusterService;
2626

27-
public ClusterViewService(EsqlFunctionRegistry functionRegistry, ClusterService clusterService) {
28-
super(functionRegistry);
27+
public ClusterViewService(EsqlFunctionRegistry functionRegistry, ClusterService clusterService, ViewServiceConfig config) {
28+
super(functionRegistry, config);
2929
this.clusterService = clusterService;
3030
}
3131

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class InMemoryViewService extends ViewService {
2222
private ViewMetadata metadata;
2323

2424
public InMemoryViewService(EsqlFunctionRegistry functionRegistry) {
25-
super(functionRegistry);
25+
super(functionRegistry, ViewServiceConfig.DEFAULT);
2626
this.metadata = ViewMetadata.EMPTY;
2727
}
2828

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

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.ResourceNotFoundException;
1111
import org.elasticsearch.action.ActionListener;
1212
import org.elasticsearch.common.Strings;
13+
import org.elasticsearch.common.settings.Settings;
1314
import org.elasticsearch.xpack.esql.VerificationException;
1415
import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
1516
import org.elasticsearch.xpack.esql.parser.EsqlParser;
@@ -27,14 +28,28 @@
2728
import java.util.function.Function;
2829

2930
public abstract class ViewService {
30-
/**
31-
* Maximum number of views referencing views referencing views.
32-
*/
33-
private static final int MAX_VIEW_DEPTH = 10;
31+
private final ViewServiceConfig config;
3432
private final EsqlFunctionRegistry functionRegistry;
3533

36-
public ViewService(EsqlFunctionRegistry functionRegistry) {
34+
public record ViewServiceConfig(int maxViews, int maxViewSize, int maxViewDepth) {
35+
36+
public static final String MAX_VIEWS_COUNT_SETTING = "esql.views.max_count";
37+
public static final String MAX_VIEWS_SIZE_SETTING = "esql.views.max_size";
38+
public static final String MAX_VIEWS_DEPTH_SETTING = "esql.views.max_depth";
39+
public static final ViewServiceConfig DEFAULT = new ViewServiceConfig(100, 10_000, 10);
40+
41+
public static ViewServiceConfig fromSettings(Settings settings) {
42+
return new ViewServiceConfig(
43+
settings.getAsInt(MAX_VIEWS_COUNT_SETTING, DEFAULT.maxViews),
44+
settings.getAsInt(MAX_VIEWS_SIZE_SETTING, DEFAULT.maxViewSize),
45+
settings.getAsInt(MAX_VIEWS_DEPTH_SETTING, DEFAULT.maxViewDepth)
46+
);
47+
}
48+
}
49+
50+
public ViewService(EsqlFunctionRegistry functionRegistry, ViewServiceConfig config) {
3751
this.functionRegistry = functionRegistry;
52+
this.config = config;
3853
}
3954

4055
protected abstract ViewMetadata getMetadata();
@@ -51,14 +66,14 @@ public LogicalPlan replaceViews(LogicalPlan plan, PlanTelemetry telemetry, Confi
5166
return ur;
5267
}
5368
View view = views.views().get(name);
54-
if (seen.size() > MAX_VIEW_DEPTH) {
55-
throw viewError("too many views referencing views ", seen);
56-
}
5769
boolean alreadySeen = seen.contains(name);
5870
seen.add(name);
5971
if (alreadySeen) {
6072
throw viewError("circular view reference ", seen);
6173
}
74+
if (seen.size() > config.maxViewDepth) {
75+
throw viewError("The maximum allowed view depth of " + config.maxViewDepth + " has been exceeded: ", seen);
76+
}
6277
return resolve(view, telemetry, configuration);
6378
});
6479
if (plan.equals(prev)) {
@@ -95,10 +110,7 @@ private VerificationException viewError(String type, List<String> seen) {
95110
*/
96111
public void put(String name, View view, ActionListener<Void> callback, Configuration configuration) {
97112
assertMasterNode();
98-
new EsqlParser().createStatement(view.query(), new QueryParams(), new PlanTelemetry(functionRegistry), configuration);
99-
// TODO should we validate this in the transport action and make it async? like plan like a query
100-
// TODO postgresql does.
101-
113+
validatePutView(name, view, configuration);
102114
updateViewMetadata(callback, current -> {
103115
Map<String, View> original = getMetadata().views();
104116
Map<String, View> updated = new HashMap<>(original);
@@ -107,6 +119,29 @@ public void put(String name, View view, ActionListener<Void> callback, Configura
107119
});
108120
}
109121

122+
private void validatePutView(String name, View view, Configuration configuration) {
123+
if (Strings.isNullOrEmpty(name)) {
124+
throw new IllegalArgumentException("name is missing or empty");
125+
}
126+
if (view == null) {
127+
throw new IllegalArgumentException("view is missing");
128+
}
129+
if (Strings.isNullOrEmpty(view.query())) {
130+
throw new IllegalArgumentException("view query is missing or empty");
131+
}
132+
if (view.query().length() > config.maxViewSize) {
133+
throw new IllegalArgumentException(
134+
"view query is too large: " + view.query().length() + " characters, the maximum allowed is " + config.maxViewSize
135+
);
136+
}
137+
if (getMetadata().views().containsKey(name) == false && getMetadata().views().size() >= config.maxViews) {
138+
throw new IllegalArgumentException("cannot add view, the maximum number of views is reached: " + config.maxViews);
139+
}
140+
new EsqlParser().createStatement(view.query(), new QueryParams(), new PlanTelemetry(functionRegistry), configuration);
141+
// TODO should we validate this in the transport action and make it async? like plan like a query
142+
// TODO postgresql does.
143+
}
144+
110145
/**
111146
* Gets the view by name.
112147
*/

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import static org.hamcrest.Matchers.containsString;
3636
import static org.hamcrest.Matchers.equalTo;
3737

38-
abstract class AbstractStatementParserTests extends ESTestCase {
38+
public abstract class AbstractStatementParserTests extends ESTestCase {
3939

4040
EsqlParser parser = new EsqlParser();
4141

@@ -53,7 +53,7 @@ LogicalPlan statement(String query, String arg) {
5353
return statement(LoggerMessageFormat.format(null, query, arg), new QueryParams());
5454
}
5555

56-
LogicalPlan statement(String e) {
56+
protected LogicalPlan statement(String e) {
5757
return statement(e, new QueryParams());
5858
}
5959

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.view;
9+
10+
import org.elasticsearch.action.ActionListener;
11+
import org.elasticsearch.xpack.esql.EsqlTestUtils;
12+
import org.elasticsearch.xpack.esql.VerificationException;
13+
import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
14+
import org.elasticsearch.xpack.esql.parser.AbstractStatementParserTests;
15+
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
16+
import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry;
17+
18+
import static org.hamcrest.Matchers.equalTo;
19+
import static org.hamcrest.Matchers.startsWith;
20+
21+
public class InMemoryViewServiceTests extends AbstractStatementParserTests {
22+
EsqlFunctionRegistry functionRegistry = new EsqlFunctionRegistry();
23+
InMemoryViewService viewService = new InMemoryViewService(functionRegistry);
24+
PlanTelemetry telemetry = new PlanTelemetry(functionRegistry);
25+
26+
public void testPutGet() throws Exception {
27+
addView("view1", "from emp");
28+
addView("view2", "from view1");
29+
addView("view3", "from view2");
30+
assertThat(viewService.get("view1").query(), equalTo("from emp"));
31+
assertThat(viewService.get("view2").query(), equalTo("from view1"));
32+
assertThat(viewService.get("view3").query(), equalTo("from view2"));
33+
}
34+
35+
public void testReplaceView() throws Exception {
36+
addView("view1", "from emp");
37+
addView("view2", "from view1");
38+
addView("view3", "from view2");
39+
LogicalPlan plan = statement("from view3");
40+
LogicalPlan rewritten = viewService.replaceViews(plan, telemetry, EsqlTestUtils.TEST_CFG);
41+
assertThat(rewritten, equalTo(statement("from emp")));
42+
}
43+
44+
public void testViewDepthExceeded() throws Exception {
45+
addView("view1", "from emp");
46+
addView("view2", "from view1");
47+
addView("view3", "from view2");
48+
addView("view4", "from view3");
49+
addView("view5", "from view4");
50+
addView("view6", "from view5");
51+
addView("view7", "from view6");
52+
addView("view8", "from view7");
53+
addView("view9", "from view8");
54+
addView("view10", "from view9");
55+
addView("view11", "from view10");
56+
LogicalPlan plan = statement("from view11");
57+
Exception e = expectThrows(VerificationException.class, () -> viewService.replaceViews(plan, telemetry, EsqlTestUtils.TEST_CFG));
58+
assertThat(e.getMessage(), startsWith("The maximum allowed view depth of 10 has been exceeded"));
59+
}
60+
61+
private void addView(String name, String query) throws Exception {
62+
viewService.put(name, new View(query), ActionListener.noop(), EsqlTestUtils.TEST_CFG);
63+
}
64+
65+
}

0 commit comments

Comments
 (0)