diff --git a/docs/changelog/141050.yaml b/docs/changelog/141050.yaml new file mode 100644 index 0000000000000..cf33a781d0384 --- /dev/null +++ b/docs/changelog/141050.yaml @@ -0,0 +1,5 @@ +area: "ES|QL" +issues: [] +pr: 141050 +summary: Add Views Security Model +type: enhancement diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java index 77e216b08121c..690a57ffd3001 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java @@ -531,7 +531,7 @@ public int size(Map lookup) { } } size += failureIndices; - } else { + } else if (IndexAbstraction.Type.DATA_STREAM.equals(indexAbstraction.getType())) { DataStream parentDataStream = (DataStream) indexAbstraction; size += parentDataStream.getFailureIndices().size(); } diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java index 40da957a75d56..1bf8f21058855 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java @@ -318,6 +318,18 @@ public void testAliases() throws Exception { } } + public void testViewRewriteDoesNotDropUnauthorizedTargetsWhenMixedWithViews() throws Exception { + expectThrows(ResponseException.class, () -> runESQLCommand("user1", "FROM index-user2 | STATS sum=sum(value)")); + expectThrows(ResponseException.class, () -> runESQLCommand("user1", "FROM index-user1,index-user2 | STATS sum=sum(value)")); + var resp = expectThrows( + ResponseException.class, + () -> runESQLCommand("user1", "FROM view-user1,index-user2 | STATS sum=sum(value)") + ); + String errorMessage = EntityUtils.toString(resp.getResponse().getEntity()); + assertThat(errorMessage, containsString("Unknown index [index-user2]")); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + public void testAliasFilter() throws Exception { for (var index : List.of("first-alias", "first-alias,index-*", "first-*,index-*")) { Response resp = runESQLCommand("alias_user1", "from " + index + " METADATA _index" + "| KEEP _index, org, value | LIMIT 10"); @@ -457,6 +469,110 @@ public void testLimitedPrivilege() throws Exception { assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); } + public void testViewRewriteDoesNotDropUnauthorizedTargets() throws Exception { + ResponseException resp = expectThrows(ResponseException.class, () -> runESQLCommand("user1", "FROM view | STATS sum=sum(value)")); + String errorMessage = EntityUtils.toString(resp.getResponse().getEntity()); + assertThat(errorMessage, containsString("unauthorized")); + assertThat(errorMessage, containsString("indices [index-user2]")); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); + } + + public void testViewRewriteAllUnauthorizedTargetsFails() throws Exception { + createView("test-admin", "other-view-user1", "FROM index-user2 | KEEP value, org"); + + ResponseException resp = expectThrows( + ResponseException.class, + () -> runESQLCommand("user1", "FROM other-view-user1 | STATS sum=sum(value)") + ); + String errorMessage = EntityUtils.toString(resp.getResponse().getEntity()); + assertThat(errorMessage, containsString("Unknown index [index-user2]")); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + public void testViewRewriteMixedUnauthorizedAndMissingTargetsFails() throws Exception { + createView("test-admin", "other-view-user1", "FROM index-user2,missing-view-target | KEEP value, org"); + + ResponseException resp = expectThrows( + ResponseException.class, + () -> runESQLCommand("user1", "FROM other-view-user1 | STATS sum=sum(value)") + ); + String errorMessage = EntityUtils.toString(resp.getResponse().getEntity()); + assertThat(errorMessage, containsString("Unknown index [index-user2,missing-view-target]")); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + public void testViewQueryAuthorized() throws Exception { + Response resp = runESQLCommand("user1", "FROM view-user1 | STATS sum=sum(value)"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat(respMap.get("columns"), equalTo(List.of(Map.of("name", "sum", "type", "double")))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(30.0d)))); + } + + public void testViewWildcardFiltersUnauthorized() throws Exception { + Response resp = runESQLCommand("user1", "FROM view-user* | STATS sum=sum(value)"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat(respMap.get("columns"), equalTo(List.of(Map.of("name", "sum", "type", "double")))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(30.0d)))); + } + + public void testNestedViewResolutionAuthorized() throws Exception { + createView("test-admin", "other-view-user1", "FROM view-user1"); + Response resp = runESQLCommand("user1", "FROM other-view-user1 | STATS sum=sum(value)"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat(respMap.get("columns"), equalTo(List.of(Map.of("name", "sum", "type", "double")))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(30.0d)))); + } + + public void testNestedViewInnerViewUnauthorized() throws Exception { + createView("test-admin", "other-view-user1", "FROM view-user2"); + ResponseException resp = expectThrows( + ResponseException.class, + () -> runESQLCommand("user1", "FROM other-view-user1 | STATS sum=sum(value)") + ); + String errorMessage = EntityUtils.toString(resp.getResponse().getEntity()); + assertThat(errorMessage, containsString("Unknown index [view-user2]")); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + public void testViewDataSelectorResolvesView() throws Exception { + Response resp = runESQLCommand("user1", "FROM view-user1::data | STATS sum=sum(value)"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat(respMap.get("columns"), equalTo(List.of(Map.of("name", "sum", "type", "double")))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(30.0d)))); + } + + public void testViewFailureSelectorNotResolved() throws Exception { + ResponseException resp = expectThrows( + ResponseException.class, + () -> runESQLCommand("user1", "FROM view-user1::failures | STATS sum=sum(value)") + ); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + public void testViewReferencingAliasAuthorized() throws Exception { + createView("test-admin", "other-view-user1", "FROM first-alias"); + Response resp = runESQLCommand("user1", "FROM other-view-user1 | STATS sum=sum(value)"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat(respMap.get("columns"), equalTo(List.of(Map.of("name", "sum", "type", "double")))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(31.0d)))); + } + + public void testViewReferencingAliasUnauthorized() throws Exception { + createView("test-admin", "other-view-user2", "FROM first-alias"); + ResponseException resp = expectThrows( + ResponseException.class, + () -> runESQLCommand("user2", "FROM other-view-user2 | STATS sum=sum(value)") + ); + String errorMessage = EntityUtils.toString(resp.getResponse().getEntity()); + assertThat(errorMessage, containsString("Unknown index [first-alias]")); + assertThat(resp.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + public void testDocumentLevelSecurity() throws Exception { Response resp = runESQLCommand("user3", "from index | stats sum=sum(value)"); assertOK(resp); diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml b/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml index fcc1f85435c51..8e634dc161d31 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml @@ -15,7 +15,7 @@ user1: - cluster:monitor/main - manage_enrich indices: - - names: ['index-user1', 'view-user1', "view", 'other-view-user1', 'index', "test-enrich" ] + - names: ['index-user1', 'view-user1', "view", 'other-view-user1', 'index', "test-enrich", 'first-alias' ] privileges: - read - write diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveViewAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveViewAction.java new file mode 100644 index 0000000000000..93b21ef167044 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlResolveViewAction.java @@ -0,0 +1,155 @@ +/* + * 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.action; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.ActionType; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.ResolvedIndexExpressions; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.action.support.TransportAction; +import org.elasticsearch.action.support.local.LocalClusterStateRequest; +import org.elasticsearch.action.support.local.TransportLocalProjectMetadataAction; +import org.elasticsearch.cluster.ProjectState; +import org.elasticsearch.cluster.block.ClusterBlockException; +import org.elasticsearch.cluster.block.ClusterBlockLevel; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.View; +import org.elasticsearch.cluster.project.ProjectResolver; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.util.concurrent.EsExecutors; +import org.elasticsearch.core.TimeValue; +import org.elasticsearch.injection.guice.Inject; +import org.elasticsearch.tasks.CancellableTask; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.tasks.TaskId; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.esql.view.ViewResolutionService; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Map; + +public class EsqlResolveViewAction extends TransportLocalProjectMetadataAction< + EsqlResolveViewAction.Request, + EsqlResolveViewAction.Response> { + public static final String NAME = "indices:data/read/esql/resolve_views"; + public static final ActionType TYPE = new ActionType<>(NAME); + + private final ViewResolutionService viewResolutionService; + + @Inject + public EsqlResolveViewAction( + TransportService transportService, + ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver, + ClusterService clusterService, + ProjectResolver projectResolver + ) { + // TODO replace DIRECT_EXECUTOR_SERVICE when removing workaround for https://github.com/elastic/elasticsearch/issues/97916 + super(NAME, actionFilters, transportService.getTaskManager(), clusterService, EsExecutors.DIRECT_EXECUTOR_SERVICE, projectResolver); + this.viewResolutionService = new ViewResolutionService(indexNameExpressionResolver); + } + + @Override + protected ClusterBlockException checkBlock(Request request, ProjectState state) { + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); + } + + @Override + protected void localClusterStateOperation(Task task, Request request, ProjectState project, ActionListener listener) { + var result = viewResolutionService.resolveViews( + project, + request.indices(), + request.indicesOptions(), + request.getResolvedIndexExpressions() + ); + listener.onResponse(new EsqlResolveViewAction.Response(result.views(), result.resolvedIndexExpressions())); + } + + public static class Request extends LocalClusterStateRequest implements IndicesRequest.Replaceable { + + private String[] indices = new String[0]; + private ResolvedIndexExpressions resolvedIndexExpressions; + private static final IndicesOptions VIEW_INDICES_OPTIONS = IndicesOptions.builder() + .wildcardOptions(IndicesOptions.WildcardOptions.builder().resolveViews(true).allowEmptyExpressions(true)) + .concreteTargetOptions(IndicesOptions.ConcreteTargetOptions.ALLOW_UNAVAILABLE_TARGETS) + .build(); + + public Request(TimeValue masterTimeout) { + super(masterTimeout); + } + + @Override + public IndicesRequest indices(String... indices) { + this.indices = indices; + return this; + } + + @Override + public Task createTask(long id, String type, String action, TaskId parentTaskId, Map headers) { + return new CancellableTask(id, type, action, getDescription(), parentTaskId, headers); + } + + @Override + public String[] indices() { + return indices; + } + + @Override + public IndicesOptions indicesOptions() { + return VIEW_INDICES_OPTIONS; + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + + @Override + public String toString() { + return "EsqlResolveViewAction.Request={indices:" + Arrays.toString(indices) + "}"; + } + + @Override + public void setResolvedIndexExpressions(ResolvedIndexExpressions expressions) { + this.resolvedIndexExpressions = expressions; + } + + @Override + public ResolvedIndexExpressions getResolvedIndexExpressions() { + return this.resolvedIndexExpressions; + } + } + + public static class Response extends ActionResponse { + private final View[] views; + private final ResolvedIndexExpressions resolvedIndexExpressions; + + public Response(View[] views, ResolvedIndexExpressions resolvedIndexExpressions) { + this.views = views; + this.resolvedIndexExpressions = resolvedIndexExpressions; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + TransportAction.localOnly(); + } + + public View[] views() { + return views; + } + + public ResolvedIndexExpressions getResolvedIndexExpressions() { + return resolvedIndexExpressions; + } + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/tree/Node.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/tree/Node.java index 6897524851d65..e757ee1774362 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/tree/Node.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/core/tree/Node.java @@ -6,6 +6,8 @@ */ package org.elasticsearch.xpack.esql.core.tree; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.SubscribableListener; import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException; import org.elasticsearch.xpack.esql.core.expression.NameId; @@ -273,6 +275,50 @@ public T transformDown(Predicate> nodePredicate, Function< return transformDown((t) -> (nodePredicate.test(t) ? rule.apply((E) t) : t)); } + /** + * Asynchronous variant of {@link #transformDown(Function)} that allows the transformation rule to perform + * async I/O operations (e.g., transport actions) without blocking the caller thread. + *

+ * Children are transformed sequentially, not concurrently, one after another in order. + * This method is intended for cases where async I/O is needed during transformation, not for parallel + * processing. + */ + @SuppressWarnings("unchecked") + public void transformDown(BiConsumer> rule, ActionListener listener) { + rule.accept((T) this, listener.delegateFailureAndWrap((originalListener, root) -> { + Node node = this.equals(root) ? this : root; + node.transformChildren((child, childListener) -> child.transformDown(rule, childListener), originalListener); + })); + } + + @SuppressWarnings("unchecked") + protected void transformChildren(BiConsumer> traversalOperation, ActionListener listener) { + if (children.isEmpty()) { + listener.onResponse((T) this); + return; + } + + final Holder> updatedChildren = new Holder<>(); + SubscribableListener chain = SubscribableListener.newForked(l -> l.onResponse(null)); + for (int i = 0; i < children.size(); i++) { + var index = i; + var child = children.get(index); + chain = chain.andThen(originalListener -> { + traversalOperation.accept(child, originalListener.delegateFailureAndWrap((o, maybeTransformed) -> { + if (maybeTransformed.equals(child) == false) { + if (updatedChildren.get() == null) { + updatedChildren.set(new ArrayList<>(children)); + } + updatedChildren.get().set(index, maybeTransformed); + } + o.onResponse(null); + })); + }); + } + chain.andThenApply(ignored -> updatedChildren.get() == null ? (T) this : replaceChildrenSameSize(updatedChildren.get())) + .addListener(listener); + } + @SuppressWarnings("unchecked") public T transformUp(Function rule) { T transformed = transformChildren(child -> child.transformUp(rule)); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java index 077b243a25102..e23b5745e09f4 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java @@ -65,6 +65,7 @@ import org.elasticsearch.xpack.esql.action.EsqlListQueriesAction; import org.elasticsearch.xpack.esql.action.EsqlQueryAction; import org.elasticsearch.xpack.esql.action.EsqlResolveFieldsAction; +import org.elasticsearch.xpack.esql.action.EsqlResolveViewAction; import org.elasticsearch.xpack.esql.action.EsqlSearchShardsAction; import org.elasticsearch.xpack.esql.action.RestEsqlAsyncQueryAction; import org.elasticsearch.xpack.esql.action.RestEsqlDeleteAsyncResultAction; @@ -271,7 +272,7 @@ public Collection createComponents(PluginServices services) { ); if (ESQL_VIEWS_FEATURE_FLAG.isEnabled()) { components = new ArrayList<>(components); - components.add(new ViewResolver(services.clusterService(), services.projectResolver())); + components.add(new ViewResolver(services.clusterService(), services.projectResolver(), services.client())); components.add(new ViewService(services.clusterService())); } return components; @@ -344,6 +345,7 @@ public List getActions() { List.of( new ActionHandler(PutViewAction.INSTANCE, TransportPutViewAction.class), new ActionHandler(DeleteViewAction.INSTANCE, TransportDeleteViewAction.class), + new ActionHandler(EsqlResolveViewAction.TYPE, EsqlResolveViewAction.class), new ActionHandler(GetViewAction.INSTANCE, TransportGetViewAction.class) ) ); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java index 4d117b900726f..ccd83239867ef 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlCCSUtils.java @@ -197,6 +197,12 @@ static void updateExecutionInfoWithUnavailableClusters( } } + /** + * Update the state for clusters that returned zero matching indices — fail the query, mark the cluster as skipped, or mark it as done. + * @param executionInfo - The per-cluster CCS state + * @param indexResolutions - The collection of IndexResolution objects produced by field-caps + * @param usedFilter - Whether the query had a request-level filter. + */ static void updateExecutionInfoWithClustersWithNoMatchingIndices( EsqlExecutionInfo executionInfo, Collection indexResolutions, @@ -218,7 +224,7 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices( * 1. fail query if no matching indices on any cluster (VerificationException) - that is handled elsewhere * 2. fail query if a cluster has no matching indices *and* a concrete index was specified - handled here */ - String fatalErrorMessage = null; + StringBuilder fatalErrorMessage = null; /* * These are clusters in the original request that are not present in the field-caps response. They were * specified with an index expression that matched no indices, so the search on that cluster is done. @@ -230,13 +236,14 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices( String error = Strings.format("Unknown index [%s]", cluster.getQualifiedIndexExpression()); if (executionInfo.shouldSkipOnFailure(c) == false || usedFilter) { if (fatalErrorMessage == null) { - fatalErrorMessage = error; + fatalErrorMessage = new StringBuilder(error); } else { - fatalErrorMessage += "; " + error; + fatalErrorMessage.append("; ").append(error); } } if (usedFilter == false) { - // We check for filter since the filter may be the reason why the index is missing, and then we don't want to mark yet + // A filter can cause field-caps to return zero indices for a pattern that actually exists. If so, we don't want to + // prematurely fail — we'll retry without the filter. markClusterWithFinalStateAndNoShards( executionInfo, c, @@ -266,8 +273,30 @@ static void updateExecutionInfoWithClustersWithNoMatchingIndices( } } } + // When views split a query into multiple branches, each branch gets its own IndexResolution. A branch for an unauthorized + // concrete index will have an empty resolution that the per-cluster check above misses. Detect these individually. + for (IndexResolution indexResolution : indexResolutions) { + if (indexResolution.isValid() + && indexResolution.resolvedIndices().isEmpty() + && concreteIndexRequested(indexResolution.get().name())) { + String clusterAlias = RemoteClusterAware.parseClusterAlias(indexResolution.get().name()); + // Already handled + if (clustersWithNoMatchingIndices.contains(clusterAlias) || executionInfo.getCluster(clusterAlias) == null) { + continue; + } + if (executionInfo.shouldSkipOnFailure(clusterAlias) == false || usedFilter) { + String error = Strings.format("Unknown index [%s]", indexResolution.get().name()); + if (fatalErrorMessage == null) { + fatalErrorMessage = new StringBuilder(error); + } else { + fatalErrorMessage.append("; ").append(error); + } + } + } + } + if (fatalErrorMessage != null) { - throw new VerificationException(fatalErrorMessage); + throw new VerificationException(fatalErrorMessage.toString()); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 0a932a84c26dc..0d705762071d6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -245,7 +245,8 @@ public void execute( parsingProfile.start(); EsqlStatement statement = parse(request); gatherSettingsMetrics(statement); - var viewResolution = viewResolver.replaceViews( + parsingProfile.stop(); + viewResolver.replaceViews( statement.plan(), (query, viewName) -> EsqlParser.INSTANCE.parseView( query, @@ -254,9 +255,22 @@ public void execute( planTelemetry, inferenceService.inferenceSettings(), viewName - ).plan() + ).plan(), + listener.delegateFailureAndWrap( + (l, viewResolution) -> analyseAndExecute(request, executionInfo, planRunner, statement, viewResolution, l) + ) ); - parsingProfile.stop(); + } + + private void analyseAndExecute( + EsqlQueryRequest request, + EsqlExecutionInfo executionInfo, + PlanRunner planRunner, + EsqlStatement statement, + ViewResolver.ViewResolutionResult viewResolution, + ActionListener> listener + ) { + assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH); PlanTimeProfile planTimeProfile = request.profile() ? new PlanTimeProfile() : null; ZoneId timeZone = request.timeZone() == null diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/DeleteViewAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/DeleteViewAction.java index 5e84b3abc2a48..fa2487909349e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/DeleteViewAction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/DeleteViewAction.java @@ -38,9 +38,9 @@ private DeleteViewAction() { } public static class Request extends AcknowledgedRequest implements IndicesRequest { - // TODO this currently doesn't support multi-target syntax, but should probably if action.destructive_requires_name=false private final String name; + // TODO: Should this match delete index request and allow for several views and `_all`? public Request(TimeValue masterNodeTimeout, TimeValue ackTimeout, String name) { super(masterNodeTimeout, ackTimeout); this.name = Objects.requireNonNull(name, "name cannot be null"); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/GetViewAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/GetViewAction.java index 2b5a89ed06d04..d0d46d142dbdc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/GetViewAction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/GetViewAction.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.ActionType; import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.ResolvedIndexExpressions; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.TransportAction; import org.elasticsearch.action.support.local.LocalClusterStateRequest; @@ -47,6 +48,7 @@ private GetViewAction() { public static class Request extends LocalClusterStateRequest implements IndicesRequest.Replaceable { private String[] indices; + private ResolvedIndexExpressions resolvedIndexExpressions; public Request(TimeValue masterNodeTimeout) { super(masterNodeTimeout); @@ -85,6 +87,16 @@ public boolean equals(Object o) { public int hashCode() { return Arrays.hashCode(indices); } + + @Override + public void setResolvedIndexExpressions(ResolvedIndexExpressions expressions) { + this.resolvedIndexExpressions = expressions; + } + + @Override + public ResolvedIndexExpressions getResolvedIndexExpressions() { + return this.resolvedIndexExpressions; + } } public static class Response extends ActionResponse implements ToXContentObject { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/TransportGetViewAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/TransportGetViewAction.java index 701f74e4573cd..400834e719522 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/TransportGetViewAction.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/TransportGetViewAction.java @@ -6,43 +6,33 @@ */ package org.elasticsearch.xpack.esql.view; -import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionType; -import org.elasticsearch.action.admin.cluster.remote.RemoteInfoResponse; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.local.TransportLocalProjectMetadataAction; import org.elasticsearch.cluster.ProjectState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; -import org.elasticsearch.cluster.metadata.ProjectId; -import org.elasticsearch.cluster.metadata.View; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.core.security.authz.IndicesAndAliasesResolverField; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.LinkedHashSet; import java.util.List; public class TransportGetViewAction extends TransportLocalProjectMetadataAction { - public static final ActionType TYPE = new ActionType<>(GetViewAction.NAME); - private final ViewService viewService; + + private final ViewResolutionService viewResolutionService; @Inject public TransportGetViewAction( TransportService transportService, ActionFilters actionFilters, + IndexNameExpressionResolver indexNameExpressionResolver, ClusterService clusterService, - ProjectResolver projectResolver, - ViewService viewService + ProjectResolver projectResolver ) { super( GetViewAction.NAME, @@ -52,7 +42,7 @@ public TransportGetViewAction( EsExecutors.DIRECT_EXECUTOR_SERVICE, projectResolver ); - this.viewService = viewService; + this.viewResolutionService = new ViewResolutionService(indexNameExpressionResolver); } @Override @@ -62,35 +52,17 @@ protected void localClusterStateOperation( ProjectState project, ActionListener listener ) { - ProjectId projectId = project.projectId(); - Collection views = new LinkedHashSet<>(); - List missing = new ArrayList<>(); - String[] names = request.indices(); - // TODO currently doesn't support wildcards when security is off - if (names == null || names.length == 0 || (names.length == 1 && Regex.isMatchAllPattern(names[0]))) { - views = viewService.getMetadata(projectId).views().values(); - } else if (Arrays.equals(names, IndicesAndAliasesResolverField.NO_INDICES_OR_ALIASES_ARRAY) == false) { - for (String name : names) { - View view = viewService.get(projectId, name); - if (view == null) { - // TODO currently doesn't throw an error when a concrete existing index is used as a view name in the API, returns empty - if (project.metadata().getIndicesLookup().containsKey(name) == false) { - missing.add(name); - } - } else { - views.add(view); - } - } - } - if (missing.isEmpty() == false) { - listener.onFailure(new ResourceNotFoundException("Views do not exist: " + String.join(", ", missing))); - } else { - listener.onResponse(new GetViewAction.Response(views)); - } + var result = viewResolutionService.resolveViews( + project, + request.indices(), + request.indicesOptions(), + request.getResolvedIndexExpressions() + ); + listener.onResponse(new GetViewAction.Response(List.of(result.views()))); } @Override protected ClusterBlockException checkBlock(GetViewAction.Request request, ProjectState state) { - return state.blocks().globalBlockedException(state.projectId(), ClusterBlockLevel.METADATA_READ); + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolutionService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolutionService.java new file mode 100644 index 0000000000000..b1b105620cd67 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolutionService.java @@ -0,0 +1,78 @@ +/* + * 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.view; + +import org.elasticsearch.action.ResolvedIndexExpression; +import org.elasticsearch.action.ResolvedIndexExpressions; +import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.cluster.ProjectState; +import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.cluster.metadata.IndexAbstractionResolver; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.View; +import org.elasticsearch.index.IndexNotFoundException; + +import java.util.List; + +import static org.elasticsearch.action.ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE; + +public class ViewResolutionService { + + private final IndexNameExpressionResolver indexNameExpressionResolver; + + public ViewResolutionService(IndexNameExpressionResolver indexNameExpressionResolver) { + this.indexNameExpressionResolver = indexNameExpressionResolver; + } + + public ViewResolutionResult resolveViews( + ProjectState projectState, + String[] indexPatterns, + IndicesOptions indicesOptions, + ResolvedIndexExpressions resolvedIndexExpressions + ) { + if (indexPatterns == null || indexPatterns.length == 0) { + return new ViewResolutionResult(new View[0], resolvedIndexExpressions); + } + + IndexAbstractionResolver indexAbstractionResolver = new IndexAbstractionResolver(indexNameExpressionResolver); + var indicesLookup = projectState.metadata().getIndicesLookup(); + + if (resolvedIndexExpressions == null) { + resolvedIndexExpressions = indexAbstractionResolver.resolveIndexAbstractions( + List.of(indexPatterns), + indicesOptions, + projectState.metadata(), + componentSelector -> indicesLookup.keySet(), + (index, selector) -> true, // Assume that a view is its own data component but has no failure component + true + ); + } + checkViewsExist(resolvedIndexExpressions, indicesOptions); + View[] views = resolvedIndexExpressions.getLocalIndicesList() + .stream() + .map(indicesLookup::get) + .filter(indexAbstraction -> indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.VIEW) + .map(indexAbstraction -> (View) indexAbstraction) + .toArray(View[]::new); + + return new ViewResolutionResult(views, resolvedIndexExpressions); + } + + private void checkViewsExist(ResolvedIndexExpressions resolvedIndexExpressions, IndicesOptions indicesOptions) { + if (indicesOptions.ignoreUnavailable()) { + return; + } + for (ResolvedIndexExpression expression : resolvedIndexExpressions.expressions()) { + if (expression.localExpressions().localIndexResolutionResult() == CONCRETE_RESOURCE_NOT_VISIBLE) { + throw new IndexNotFoundException(expression.original()); + } + } + } + + public record ViewResolutionResult(View[] views, ResolvedIndexExpressions resolvedIndexExpressions) {} +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolver.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolver.java index fdcbea9408fe9..8214c6e344b5c 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolver.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/view/ViewResolver.java @@ -7,7 +7,10 @@ package org.elasticsearch.xpack.esql.view; -import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ResolvedIndexExpression; +import org.elasticsearch.action.support.SubscribableListener; +import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.metadata.View; import org.elasticsearch.cluster.metadata.ViewMetadata; import org.elasticsearch.cluster.project.ProjectResolver; @@ -19,6 +22,7 @@ import org.elasticsearch.logging.Logger; import org.elasticsearch.xpack.core.esql.EsqlFeatureFlags; import org.elasticsearch.xpack.esql.VerificationException; +import org.elasticsearch.xpack.esql.action.EsqlResolveViewAction; import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Fork; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; @@ -27,15 +31,16 @@ import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; import java.util.ArrayList; -import java.util.Collection; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.function.BiFunction; -import java.util.stream.Collectors; +import java.util.function.Predicate; + +import static org.elasticsearch.rest.RestUtils.REST_MASTER_TIMEOUT_DEFAULT; public class ViewResolver { @@ -43,6 +48,7 @@ public class ViewResolver { private final ClusterService clusterService; private final ProjectResolver projectResolver; private volatile int maxViewDepth; + private final Client client; public static final Setting MAX_VIEW_DEPTH_SETTING = Setting.intSetting( "esql.views.max_view_depth", 10, @@ -59,11 +65,13 @@ public ViewResolver() { this.clusterService = null; this.projectResolver = null; this.maxViewDepth = 0; + this.client = null; } - public ViewResolver(ClusterService clusterService, ProjectResolver projectResolver) { + public ViewResolver(ClusterService clusterService, ProjectResolver projectResolver, Client client) { this.clusterService = clusterService; this.projectResolver = projectResolver; + this.client = client; clusterService.getClusterSettings().initializeAndWatch(MAX_VIEW_DEPTH_SETTING, v -> this.maxViewDepth = v); } @@ -71,10 +79,6 @@ ViewMetadata getMetadata() { return clusterService.state().metadata().getProject(projectResolver.getProjectId()).custom(ViewMetadata.TYPE, ViewMetadata.EMPTY); } - protected Map getIndicesLookup() { - return clusterService.state().metadata().getProject(projectResolver.getProjectId()).getIndicesLookup(); - } - protected boolean viewsFeatureEnabled() { return EsqlFeatureFlags.ESQL_VIEWS_FEATURE_FLAG.isEnabled(); } @@ -85,225 +89,296 @@ protected boolean viewsFeatureEnabled() { public record ViewResolutionResult(LogicalPlan plan, Map viewQueries) {} /** - * Replaces views in the plan with their resolved definitions. - * @param plan the plan to resolve views in - * @param parser a function that parses a view query with a given view name - * The BiFunction takes (query, viewName) and returns the parsed LogicalPlan. - * The viewName is used to tag Source objects so they can be correctly deserialized. - * @return the resolution result containing the rewritten plan and collected view queries + * Replaces views in the logical plan with their subqueries recursively. + *

+ * This method performs a depth-first, top-down (pre-order) traversal of the plan tree. + * During traversal, it intercepts specific node types: + *

    + *
  • {@code UnresolvedRelation}: Resolves views and replaces them with their query plans, + * then recursively processes those plans
    • + *
  • {@code Fork}: Recursively processes each child branch
    • + *
  • {@code UnionAll}: Skipped (assumes rewriting is already complete)
    • + *
+ *

+ * View resolution may introduce new nodes that need further processing, so explicit + * recursive calls are made on newly resolved view plans. The method tracks circular + * references and enforces maximum view depth limits. + * + * @param plan the logical plan to process + * @param parser function to parse view query strings into logical plans + * @param listener callback that receives the rewritten plan and a map of view names to their queries */ - public ViewResolutionResult replaceViews(LogicalPlan plan, BiFunction parser) { - if (viewsFeatureEnabled() == false) { - return new ViewResolutionResult(plan, Map.of()); - } - ViewMetadata views = getMetadata(); - if (views.views().isEmpty()) { - // Don't bother to traverse the plan if there are no views defined - return new ViewResolutionResult(plan, Map.of()); - } - // Get all non-view names for this project, so we know if wildcards match any non-view indexes - Set nonViewNames = getIndicesLookup().entrySet() - .stream() - .filter(e -> e.getValue() == null || e.getValue().getType() != IndexAbstraction.Type.VIEW) - .map(Map.Entry::getKey) - .collect(Collectors.toSet()); + public void replaceViews( + LogicalPlan plan, + BiFunction parser, + ActionListener listener + ) { Map viewQueries = new HashMap<>(); - LogicalPlan rewritten = replaceViewsInSubplan( + if (viewsFeatureEnabled() == false || getMetadata().views().isEmpty()) { + listener.onResponse(new ViewResolutionResult(plan, viewQueries)); + return; + } + replaceViews( plan, parser, - views, - nonViewNames, new LinkedHashSet<>(), - new HashSet<>(), + viewQueries, 0, - viewQueries + listener.delegateFailureAndWrap((l, rewritten) -> listener.onResponse(new ViewResolutionResult(rewritten, viewQueries))) ); - if (rewritten.equals(plan)) { - log.debug("No views resolved"); - return new ViewResolutionResult(plan, Map.of()); - } - log.debug("Views resolved:\n" + rewritten); - return new ViewResolutionResult(rewritten, viewQueries); } - /** - * This method uses recursion to handle branched plans (Fork, Union, Subqueries, Views), while also using transformDown to handle - * linear plans. TransformDown is also a recursive method, so this results in a depth-first traversal of the plan tree. - * We maintain the same "seen" set for each branch so that multiple branches can refer to the same view without causing - * false circular reference errors. - */ - private LogicalPlan replaceViewsInSubplan( + private void replaceViews( LogicalPlan plan, BiFunction parser, - ViewMetadata views, - Set nonViewNames, - LinkedHashSet outerSeen, - HashSet outerSeenWildcards, + LinkedHashSet seenViews, + Map viewQueries, int depth, - Map viewQueries + ActionListener listener ) { - // Do not modify the outer seen set, copy it for this subplan, allowing multiple subplans to refer to the same view - LinkedHashSet seen = new LinkedHashSet<>(outerSeen); - HashSet seenWildcards = new HashSet<>(outerSeenWildcards); - String tab = " ".repeat(depth); - String pt = " " + tab; - log.trace( - tab + "replaceViewsInSubplan depth=" + depth + " seen=" + seen + " plan=\n" + pt + plan.toString().replace("\n", "\n" + pt) - ); - LogicalPlan rewritten = plan.transformDown(LogicalPlan.class, p -> { + LinkedHashSet seenInner = new LinkedHashSet<>(seenViews); + // Tracks wildcard patterns already resolved within this transformDown traversal to prevent duplicate processing + HashSet seenWildcards = new HashSet<>(); + + plan.transformDown((p, planListener) -> { switch (p) { case UnionAll union -> { // UnionAll is the result of this re-writing, so we assume rewriting is completed // TODO: This could conflicts with subquery feature, perhaps we need a new plan node type? - return union; + planListener.onResponse(union); + return; } case Fork fork -> { - List subplans = new ArrayList<>(fork.children()); - boolean changed = false; - for (int i = 0; i < subplans.size(); i++) { - LogicalPlan subplan = replaceViewsInSubplan( - subplans.get(i), - parser, - views, - nonViewNames, - seen, - seenWildcards, - depth + 1, - viewQueries - ); - if (subplan.equals(subplans.get(i)) == false) { - changed = true; - subplans.set(i, subplan); - } - } - if (changed) { - return new Fork(fork.source(), subplans, fork.output()); - } - return fork; + replaceViewsFork(fork, parser, seenInner, viewQueries, depth, planListener); + return; } case UnresolvedRelation ur -> { - List subqueries = new ArrayList<>(); - List indexes = new ArrayList<>(); - IndexPatterns patterns = extractViewAndIndexNames(views, ur, nonViewNames, seenWildcards); - if (patterns.views().isEmpty()) { - // No views found, return the original plan node - return ur; - } - log.trace(tab + " found UnresolvedRelation with views: " + patterns.views().stream().map(View::name).toList()); - for (View view : patterns.views()) { - if (seen.add(view.name()) == false) { - throw viewError("circular view reference '" + view.name() + "': ", new ArrayList<>(seen)); - } - if (seen.size() > this.maxViewDepth) { - throw viewError("The maximum allowed view depth of " + this.maxViewDepth + " has been exceeded: ", seen); - } - LogicalPlan resolvedView = resolve(view, parser, viewQueries); - subqueries.add( - new ViewPlan( - view.name(), - replaceViewsInSubplan( - resolvedView, - parser, - views, - nonViewNames, - seen, - seenWildcards, - depth + 1, - viewQueries - ) - ) - ); - } - indexes.addAll(patterns.indexNames()); - indexes.addAll(patterns.wildCards()); - if (indexes.isEmpty()) { - if (subqueries.size() == 1) { - // only one view, no need for union, return view plan directly - return subqueries.getFirst().plan; - } - } else { - // We have non-view indexes, so we need an UnresolvedRelation for them too - subqueries.addFirst( - new ViewPlan( - null, - new UnresolvedRelation( - ur.source(), - new IndexPattern(ur.indexPattern().source(), String.join(",", indexes)), - ur.frozen(), - ur.metadataFields(), - ur.indexMode(), - ur.unresolvedMessage() - ) - ) - ); - } - // We replace the UnresolvedRelation with a UnionAll of all the view subqueries (and possibly an UnresolvedRelation) - return createTopPlan(ur, subqueries, depth); + replaceViewsUnresolvedRelation(ur, parser, seenInner, seenWildcards, viewQueries, depth, planListener); + return; } default -> { } } - // All other plan types are returned unchanged - // TODO: determine if we need to modify source fields to resolve deserialization issues - return p; - }); - log.trace( - tab + "rewritten plan at depth=" + depth + " seen=" + seen + " is\n" + pt + rewritten.toString().replace("\n", "\n" + pt) - ); - return rewritten; + planListener.onResponse(p); + }, listener); } - private record IndexPatterns(List views, List indexNames, List wildCards) {} + private void replaceViewsFork( + Fork fork, + BiFunction parser, + LinkedHashSet seenViews, + Map viewQueries, + int depth, + ActionListener listener + ) { + var currentSubplans = fork.children(); + SubscribableListener> chain = SubscribableListener.newForked(l -> l.onResponse(null)); + for (int i = 0; i < currentSubplans.size(); i++) { + var index = i; + var subplan = currentSubplans.get(i); + chain = chain.andThen( + (l, updatedSubplans) -> replaceViews( + subplan, + parser, + seenViews, + viewQueries, + depth + 1, + l.delegateFailureAndWrap((subListener, newPlan) -> { + if (newPlan.equals(subplan) == false) { + var updatedSubplansInner = updatedSubplans; + if (updatedSubplansInner == null) { + updatedSubplansInner = new ArrayList<>(currentSubplans); + } + updatedSubplansInner.set(index, newPlan); + subListener.onResponse(updatedSubplansInner); + } else { + subListener.onResponse(updatedSubplans); + } + }) + ) + ); + } + chain.andThenApply(updatedSubplans -> { + if (updatedSubplans != null) { + return new Fork(fork.source(), updatedSubplans, fork.output()); + } + return (LogicalPlan) fork; + }).addListener(listener); + } - /** - * Extract view names from an UnresolvedRelation, expanding any wildcards. - * This method also returns the original names (including wildcards) so that indexes can be resolved later. - */ - private IndexPatterns extractViewAndIndexNames( - ViewMetadata viewMetadata, + private void replaceViewsUnresolvedRelation( UnresolvedRelation unresolvedRelation, - Set nonViewNames, - HashSet seenWildcards + BiFunction parser, + LinkedHashSet seenViews, + HashSet seenWildcards, + Map viewQueries, + int depth, + ActionListener listener ) { - List views = new ArrayList<>(); - List indexNames = new ArrayList<>(); - List wildCards = new ArrayList<>(); - for (String name : unresolvedRelation.indexPattern().indexPattern().split(",")) { - name = name.trim(); - // We do not allow remote cluster ':' specifications for views - if (Regex.isSimpleMatchPattern(name) && name.contains(":") == false) { - if (seenWildcards.contains(name)) { - continue; // already processed this wildcard + // Avoid re-resolving wildcards preserved for non-view matches in subsequent transformDown visits. + var patterns = Arrays.stream(unresolvedRelation.indexPattern().indexPattern().split(",")) + .filter(pattern -> Regex.isSimpleMatchPattern(pattern) == false || seenWildcards.contains(pattern) == false) + .toArray(String[]::new); + for (String pattern : patterns) { + if (Regex.isSimpleMatchPattern(pattern)) { + seenWildcards.add(pattern); + } + } + + var req = new EsqlResolveViewAction.Request(REST_MASTER_TIMEOUT_DEFAULT); + req.indices(patterns); + + doEsqlResolveViewsRequest(req, listener.delegateFailureAndWrap((l1, response) -> { + if (response.views().length == 0) { + listener.onResponse(stripValidConcreteViewExclusions(unresolvedRelation, patterns)); + return; + } + + final List subqueries = new ArrayList<>(); + SubscribableListener chain = SubscribableListener.newForked(l2 -> l2.onResponse(null)); + for (var view : response.views()) { + chain = chain.andThen(l2 -> { + validateViewReferenceAndMarkSeen(view.name(), seenViews); + replaceViews( + resolve(view, parser, viewQueries), + parser, + seenViews, + viewQueries, + depth + 1, + l2.delegateFailureAndWrap((l3, fullyResolved) -> { + subqueries.add(new ViewPlan(view.name(), fullyResolved)); + l3.onResponse(null); + }) + ); + }); + } + chain.andThenApply(ignored -> { + var unresolvedPatterns = buildUnresolvedPatterns(response, seenViews, patterns); + if (unresolvedPatterns.isEmpty() && subqueries.size() == 1) { + // only one view, no need for UnionAll, return view plan directly + return subqueries.getFirst().plan(); } - seenWildcards.add(name); - // If the name includes a wildcard, expand it to all matching views, - // leaving the original wildcard name in place for index resolution - for (String viewName : viewMetadata.views().keySet()) { - if (Regex.simpleMatch(name, viewName)) { - views.add(viewMetadata.getView(viewName)); - } + if (unresolvedPatterns.isEmpty() == false) { + // We have non-view indexes, so we need an UnresolvedRelation for them too + subqueries.add(createUnresolvedRelationPlan(unresolvedRelation, unresolvedPatterns)); } - // If there exist local indices matching the wildcard, keep the wildcard for later index resolution - // TODO: See how to generalize this for CCS and CPS, probably need additional field-caps calls - for (String indexName : nonViewNames) { - if (Regex.simpleMatch(name, indexName)) { - // The existence of any non-view index matching the wildcard means we need to keep the wildcard - wildCards.add(name); - break; - } + return buildPlanFromBranches(unresolvedRelation, subqueries, depth); + }).addListener(listener); + })); + } + + private void validateViewReferenceAndMarkSeen(String viewName, LinkedHashSet seenViews) { + if (seenViews.add(viewName) == false) { + throw new VerificationException("circular view reference '" + viewName + "': " + String.join(" -> ", seenViews)); + } + if (seenViews.size() > this.maxViewDepth) { + throw new VerificationException( + "The maximum allowed view depth of " + this.maxViewDepth + " has been exceeded: " + String.join(" -> ", seenViews) + ); + } + } + + /** + * Builds the list of unresolved (non-view) patterns from the view resolution response. + *

+ * Expressions marked as {@code CONCRETE_RESOURCE_NOT_VISIBLE}, {@code CONCRETE_RESOURCE_UNAUTHORIZED} or isn't a view flows through to + * field caps. There they either fail via the same security checks that handle non-view queries (a search) or are resolved to a non-view + * resource. Exclusion patterns from the original query that target non-view resources are also preserved. This ensures that + * index-level exclusions are re-applied during the later index resolution step. + */ + private List buildUnresolvedPatterns( + EsqlResolveViewAction.Response response, + LinkedHashSet seenViews, + String[] originalPatterns + ) { + List unresolvedPatterns = new ArrayList<>(); + for (var resolvedIndexExpression : response.getResolvedIndexExpressions().expressions()) { + var result = resolvedIndexExpression.localExpressions().localIndexResolutionResult(); + // If any concrete resource (view, alias, datastream or index) was unauthorized, pass it along as an unresolved relation + if (result == ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_NOT_VISIBLE + || result == ResolvedIndexExpression.LocalIndexResolutionResult.CONCRETE_RESOURCE_UNAUTHORIZED) { + unresolvedPatterns.add(resolvedIndexExpression.original()); + continue; + } + // If any of the concrete resources were not views, pass them along as an unresolved relation + if (resolvedIndexExpression.localExpressions().indices().stream().anyMatch(index -> seenViews.contains(index) == false)) { + unresolvedPatterns.add(resolvedIndexExpression.original()); + } + } + if (unresolvedPatterns.isEmpty() == false) { + var viewNames = getMetadata().views(); + for (String pattern : originalPatterns) { + if (patternIsExclusion(pattern) && isConcreteViewExclusion(pattern, viewNames::containsKey) == false) { + unresolvedPatterns.add(pattern); } - } else if (viewMetadata.getView(name) != null) { - views.add(viewMetadata.getView(name)); - } else { - indexNames.add(name); } } - return new IndexPatterns(views, indexNames, wildCards); + return unresolvedPatterns; + } + + /** + * Checks whether a pattern is an exclusion targeting a concrete (non-wildcard) view name. + */ + private static boolean isConcreteViewExclusion(String pattern, Predicate viewExistsPredicate) { + if (patternIsExclusion(pattern) == false) { + return false; + } + String target = pattern.substring(1); + return Regex.isSimpleMatchPattern(target) == false && viewExistsPredicate.test(target); + } + + private static boolean patternIsExclusion(String pattern) { + return pattern.startsWith("-"); + } + + /** + * Returns a copy of the unresolved relation with concrete view exclusions removed from its pattern. + * Used in the early return path when no views were resolved, to prevent valid view exclusions from + * reaching field caps where they would fail. + */ + private UnresolvedRelation stripValidConcreteViewExclusions(UnresolvedRelation ur, String[] patterns) { + var viewNames = getMetadata().views(); + var filtered = Arrays.stream(patterns) + .filter(p -> isConcreteViewExclusion(p, viewNames::containsKey) == false) + .toArray(String[]::new); + if (filtered.length == patterns.length) { + return ur; + } + return new UnresolvedRelation( + ur.source(), + new IndexPattern(ur.indexPattern().source(), String.join(",", filtered)), + ur.frozen(), + ur.metadataFields(), + ur.indexMode(), + ur.unresolvedMessage() + ); + } + + private ViewPlan createUnresolvedRelationPlan(UnresolvedRelation ur, List unresolvedPatterns) { + return new ViewPlan( + null, + new UnresolvedRelation( + ur.source(), + new IndexPattern(ur.indexPattern().source(), String.join(",", unresolvedPatterns)), + ur.frozen(), + ur.metadataFields(), + ur.indexMode(), + ur.unresolvedMessage() + ) + ); + } + + // Visible for testing + protected void doEsqlResolveViewsRequest( + EsqlResolveViewAction.Request request, + ActionListener listener + ) { + client.execute(EsqlResolveViewAction.TYPE, request, listener); } record ViewPlan(String name, LogicalPlan plan) {} - private LogicalPlan createTopPlan(UnresolvedRelation ur, List subqueries, int depth) { + private LogicalPlan buildPlanFromBranches(UnresolvedRelation ur, List subqueries, int depth) { List unresolvedRelations = new ArrayList<>(); List otherPlans = new ArrayList<>(); for (ViewPlan lp : subqueries) { @@ -331,13 +406,20 @@ private LogicalPlan createTopPlan(UnresolvedRelation ur, List subqueri if (otherPlans.size() == 1) { return otherPlans.getFirst(); } + traceUnionAllBranches(depth, otherPlans); + return new UnionAll(ur.source(), otherPlans, List.of()); + } + + private void traceUnionAllBranches(int depth, List plans) { + if (log.isTraceEnabled() == false) { + return; + } String tab = " ".repeat(depth); - log.trace(tab + " creating UnionAll with " + otherPlans.size() + " branches:"); - String pt = " " + tab; - for (LogicalPlan p : otherPlans) { - log.trace(tab + " branch plan=\n" + pt + p.toString().replace("\n", "\n" + pt)); + log.trace("{} creating UnionAll with {} branches:", tab, plans.size()); + String branchPrefix = " " + tab; + for (LogicalPlan p : plans) { + log.trace("{} branch plan=\n{}{}", tab, branchPrefix, p.toString().replace("\n", "\n" + branchPrefix)); } - return new UnionAll(ur.source(), otherPlans, List.of()); } private LogicalPlan resolve(View view, BiFunction parser, Map viewQueries) { @@ -349,17 +431,4 @@ private LogicalPlan resolve(View view, BiFunction p // to be tagged with the view name during parsing return parser.apply(view.query(), view.name()); } - - private VerificationException viewError(String type, Collection seen) { - StringBuilder b = new StringBuilder(); - for (String s : seen) { - if (b.isEmpty()) { - b.append(type); - } else { - b.append(" -> "); - } - b.append(s); - } - throw new VerificationException(b.toString()); - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index d02fb6b92ccf4..baf00b3311207 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -117,6 +117,7 @@ import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import org.elasticsearch.xpack.esql.view.InMemoryViewService; import org.elasticsearch.xpack.esql.view.PutViewAction; +import org.elasticsearch.xpack.esql.view.ViewResolver; import org.junit.After; import org.junit.AssumptionViolatedException; import org.junit.Before; @@ -655,16 +656,19 @@ private LogicalPlan resolveViews(LogicalPlan parsed) { if (shouldLoadViews() == false) { return parsed; } + try (InMemoryViewService viewService = InMemoryViewService.makeViewService()) { for (var viewConfig : VIEW_CONFIGS.values()) { loadView(viewService, viewConfig); } - return viewService.getViewResolver().replaceViews(parsed, this::parseView).plan(); + PlainActionFuture future = new PlainActionFuture<>(); + viewService.getViewResolver().replaceViews(parsed, this::parseView, future); + return future.actionGet().plan(); } } private void loadView(InMemoryViewService viewService, CsvTestsDataLoader.ViewConfig viewConfig) { - ProjectId projectId = ProjectId.fromId("dummy"); + ProjectId projectId = ProjectId.DEFAULT; PutViewAction.Request request = new PutViewAction.Request( TimeValue.ONE_MINUTE, TimeValue.ONE_MINUTE, diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/NodeTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/NodeTests.java index 7c738ee42c921..69c57e3b07043 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/NodeTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/core/tree/NodeTests.java @@ -6,6 +6,9 @@ */ package org.elasticsearch.xpack.esql.core.tree; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.LatchedActionListener; +import org.elasticsearch.action.support.ActionTestUtils; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException; @@ -17,6 +20,11 @@ import java.util.LinkedList; import java.util.List; import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BiConsumer; +import java.util.function.Consumer; import static java.util.Collections.singletonList; import static org.elasticsearch.xpack.esql.core.tree.SourceTests.randomSource; @@ -61,6 +69,62 @@ public void testWithImmutableChildList() { assertEquals(node.children().size(), 0); } + public void testTransformDownAsyncNoChildren() throws InterruptedException { + NoChildren node = new NoChildren(randomSource(), "original"); + + assertAsyncTransform(node, (n, listener) -> { + NoChildren transformed = new NoChildren(n.source(), "transformed"); + listener.onResponse(transformed); + }, result -> { + assertEquals(NoChildren.class, result.getClass()); + assertEquals("transformed", result.thing()); + }); + } + + public void testTransformDownAsyncWithChildren() throws InterruptedException { + NoChildren child1 = new NoChildren(randomSource(), "child1"); + NoChildren child2 = new NoChildren(randomSource(), "child2"); + ChildrenAreAProperty parent = new ChildrenAreAProperty(randomSource(), Arrays.asList(child1, child2), "parent"); + + assertAsyncTransform(parent, (n, listener) -> { + if (n instanceof NoChildren) { + NoChildren nc = (NoChildren) n; + if ("child1".equals(nc.thing())) { + listener.onResponse(new NoChildren(nc.source(), "transformed1")); + } else { + listener.onResponse(n); + } + } else { + listener.onResponse(n); + } + }, result -> { + assertEquals(ChildrenAreAProperty.class, result.getClass()); + ChildrenAreAProperty transformed = (ChildrenAreAProperty) result; + assertEquals(2, transformed.children().size()); + assertEquals("transformed1", transformed.children().get(0).thing()); + assertEquals("child2", transformed.children().get(1).thing()); + }); + } + + public void testTransformDownAsyncNoChange() throws InterruptedException { + NoChildren node = new NoChildren(randomSource(), "unchanged"); + assertAsyncTransform(node, (n, listener) -> listener.onResponse(n), result -> assertSame(node, result)); + } + + private void assertAsyncTransform(Dummy node, BiConsumer> rule, Consumer assertions) + throws InterruptedException { + CountDownLatch latch = new CountDownLatch(1); + AtomicBoolean listenerCalled = new AtomicBoolean(false); + + LatchedActionListener listener = new LatchedActionListener<>(ActionTestUtils.assertNoFailureListener(result -> { + assertTrue("listener called more than once", listenerCalled.compareAndSet(false, true)); + assertions.accept(result); + }), latch); + + node.transformDown(rule, listener); + assertTrue("timed out after 5s", latch.await(5, TimeUnit.SECONDS)); + } + public abstract static class Dummy extends Node { private final String thing; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewResolver.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewResolver.java index e2aa18969a484..fcf554e8fd0af 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewResolver.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewResolver.java @@ -7,22 +7,42 @@ package org.elasticsearch.xpack.esql.view; -import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.ViewMetadata; +import org.elasticsearch.cluster.project.DefaultProjectResolver; +import org.elasticsearch.cluster.project.ProjectResolver; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.indices.EmptySystemIndices; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.esql.action.EsqlResolveViewAction; -import java.util.LinkedHashSet; -import java.util.Map; +import java.util.Set; import java.util.function.Supplier; -import java.util.stream.Collectors; + +import static org.mockito.Mockito.mock; public class InMemoryViewResolver extends ViewResolver { protected Supplier metadata; - protected LinkedHashSet indices = new LinkedHashSet<>(); + protected IndexNameExpressionResolver indexNameExpressionResolver; + protected ClusterService clusterService; + protected ProjectResolver projectResolver; public InMemoryViewResolver(ClusterService clusterService, Supplier metadata) { - super(clusterService, null); + super(clusterService, null, null); + this.projectResolver = DefaultProjectResolver.INSTANCE; + this.indexNameExpressionResolver = new IndexNameExpressionResolver( + new ThreadContext(Settings.EMPTY), + EmptySystemIndices.INSTANCE, + projectResolver + ); this.metadata = metadata; + this.clusterService = clusterService; + } @Override @@ -30,28 +50,27 @@ protected ViewMetadata getMetadata() { return metadata.get(); } - @Override - protected Map getIndicesLookup() { - Map viewsLookup = getMetadata().views() - .values() - .stream() - .collect(Collectors.toMap(IndexAbstraction::getName, v -> v)); - for (String index : indices) { - viewsLookup.put(index, null); - } - return viewsLookup; - } - protected boolean viewsFeatureEnabled() { // This is a test implementation, so we assume the feature is always enabled return true; } - public void addIndex(String name) { - indices.add(name); + @Override + protected void doEsqlResolveViewsRequest( + EsqlResolveViewAction.Request request, + ActionListener listener + ) { + var action = new EsqlResolveViewAction( + mock(TransportService.class), + new ActionFilters(Set.of()), + indexNameExpressionResolver, + clusterService, + projectResolver + ); + action.execute(mock(Task.class), request, listener); } - public void clear() { - indices.clear(); + public void close() { + clusterService.close(); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java index 8e537db9caf41..66299ccdfc557 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewService.java @@ -9,7 +9,9 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.cluster.metadata.View; @@ -18,17 +20,21 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.test.ClusterServiceUtils; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; import java.io.Closeable; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import static org.elasticsearch.test.ESTestCase.indexSettings; import static org.elasticsearch.xpack.esql.view.ViewResolver.MAX_VIEW_DEPTH_SETTING; /** @@ -38,7 +44,9 @@ public class InMemoryViewService extends ViewService implements Closeable { private final ThreadPool threadPool; - protected ViewMetadata metadata; + private ViewMetadata viewMetadata; + private final List indices = new ArrayList<>(); + private static final Set> ALL_SETTINGS; static { Set> settings = new HashSet<>(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); @@ -64,26 +72,26 @@ private static InMemoryViewService makeViewService(ThreadPool threadPool, Settin private InMemoryViewService(ClusterService clusterService, ThreadPool threadPool, ViewMetadata metadata) { super(clusterService); this.threadPool = threadPool; - this.metadata = metadata; + this.viewMetadata = metadata; } InMemoryViewService withSettings(Settings settings) { - return InMemoryViewService.makeViewService(settings, this.metadata); + return InMemoryViewService.makeViewService(settings, this.viewMetadata); } @Override protected ViewMetadata getMetadata(ProjectMetadata projectMetadata) { - return metadata; + return viewMetadata; } @Override protected ViewMetadata getMetadata(ProjectId projectId) { - return metadata; + return viewMetadata; } @Override protected Map getIndicesLookup(ProjectMetadata projectMetadata) { - return metadata.views().values().stream().collect(Collectors.toMap(IndexAbstraction::getName, v -> v)); + return viewMetadata.views().values().stream().collect(Collectors.toMap(IndexAbstraction::getName, v -> v)); } @Override @@ -91,21 +99,39 @@ public void putView(ProjectId projectId, PutViewAction.Request request, ActionLi try { // Validate the way we would normally validate in ViewService validatePutView(null, request.view()); - Map existingViews = new HashMap<>(metadata.views()); + Map existingViews = new HashMap<>(viewMetadata.views()); existingViews.put(request.view().name(), request.view()); - metadata = new ViewMetadata(existingViews); + viewMetadata = new ViewMetadata(existingViews); + var projectBuilder = ProjectMetadata.builder(projectId).putCustom(ViewMetadata.TYPE, viewMetadata); + indices.forEach( + index -> projectBuilder.put(IndexMetadata.builder(index).settings(indexSettings(IndexVersion.current(), 1, 0))) + ); + ClusterServiceUtils.setState( + clusterService, + new ClusterState.Builder(clusterService.state()).putProjectMetadata(projectBuilder).build() + ); listener.onResponse(AcknowledgedResponse.TRUE); } catch (Exception e) { listener.onFailure(e); } } + public void addIndex(ProjectId projectId, String name) { + var projectBuilder = ProjectMetadata.builder(projectId).putCustom(ViewMetadata.TYPE, viewMetadata); + indices.add(name); + indices.forEach(index -> projectBuilder.put(IndexMetadata.builder(index).settings(indexSettings(IndexVersion.current(), 1, 0)))); + ClusterServiceUtils.setState( + clusterService, + new ClusterState.Builder(clusterService.state()).putProjectMetadata(projectBuilder).build() + ); + } + @Override public void deleteView(ProjectId projectId, DeleteViewAction.Request request, ActionListener listener) { try { - Map existingViews = new HashMap<>(metadata.views()); + Map existingViews = new HashMap<>(viewMetadata.views()); existingViews.remove(request.name()); - metadata = new ViewMetadata(existingViews); + viewMetadata = new ViewMetadata(existingViews); listener.onResponse(AcknowledgedResponse.TRUE); } catch (Exception e) { listener.onFailure(e); @@ -122,14 +148,18 @@ public void close() { if (this.threadPool != null) { this.threadPool.shutdownNow(); } + if (clusterService != null) { + this.clusterService.close(); + } } // Used for testing purposes - void clearAllViews() { - metadata = ViewMetadata.EMPTY; + void clearAllViewsAndIndices() { + viewMetadata = ViewMetadata.EMPTY; + indices.clear(); } public InMemoryViewResolver getViewResolver() { - return new InMemoryViewResolver(clusterService, () -> metadata); + return new InMemoryViewResolver(clusterService, () -> viewMetadata); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java index 9e307df31aca2..a064185fc5536 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/InMemoryViewServiceTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.esql.view; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.View; import org.elasticsearch.common.settings.Settings; @@ -24,13 +25,16 @@ import org.elasticsearch.xpack.esql.inference.InferenceSettings; import org.elasticsearch.xpack.esql.io.stream.PlanStreamOutput; import org.elasticsearch.xpack.esql.parser.AbstractStatementParserTests; +import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.parser.QueryParams; import org.elasticsearch.xpack.esql.plan.SettingsValidationContext; import org.elasticsearch.xpack.esql.plan.logical.EsRelationSerializationTests; import org.elasticsearch.xpack.esql.plan.logical.Eval; +import org.elasticsearch.xpack.esql.plan.logical.Fork; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.Subquery; import org.elasticsearch.xpack.esql.plan.logical.UnionAll; +import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; import org.elasticsearch.xpack.esql.session.Configuration; import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry; import org.hamcrest.BaseMatcher; @@ -40,15 +44,23 @@ import org.junit.Before; import org.junit.BeforeClass; +import java.time.Instant; +import java.time.LocalDate; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Objects; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.as; +import static org.elasticsearch.xpack.esql.EsqlTestUtils.equalToIgnoringIds; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; @@ -57,10 +69,15 @@ import static org.hamcrest.Matchers.startsWith; public class InMemoryViewServiceTests extends AbstractStatementParserTests { + protected final EsqlParser parser = EsqlParser.INSTANCE; + private static final InferenceSettings EMPTY_INFERENCE_SETTINGS = new InferenceSettings(Settings.EMPTY); static InMemoryViewService viewService; static InMemoryViewResolver viewResolver; + PlanTelemetry telemetry = new PlanTelemetry(new EsqlFunctionRegistry()); + QueryParams queryParams = new QueryParams(); + ProjectId projectId = ProjectId.DEFAULT; @BeforeClass public static void setup() { @@ -75,14 +92,12 @@ public static void afterTearDown() { @Before public void setupTest() { - viewService.clearAllViews(); - viewResolver.clear(); + viewService.clearAllViewsAndIndices(); + for (String idx : List.of("emp", "emp1", "emp2", "emp3", "logs")) { + addIndex(idx); + } } - PlanTelemetry telemetry = new PlanTelemetry(new EsqlFunctionRegistry()); - QueryParams queryParams = new QueryParams(); - ProjectId projectId = ProjectId.fromId("1"); - public void testPutGet() { addView("view1", "FROM emp"); addView("view2", "FROM view1"); @@ -97,8 +112,187 @@ public void testReplaceView() { addView("view2", "FROM view1"); addView("view3", "FROM view2"); LogicalPlan plan = query("FROM view3"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp"))); + } + + public void testViewExclusion() { + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + LogicalPlan plan = query("FROM view*, -view2"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1"))); + } + + public void testExclusionWithRemainingIndexMatch() { + addView("logs-nginx", "FROM logs-1 | WHERE logs.type == nginx"); + addIndex("logs-1"); + LogicalPlan plan = query("FROM logs*, -logs-nginx"); + assertThat(replaceViews(plan), matchesPlan(query("FROM logs*"))); + } + + public void testExclusionWithDuplicateViewWildcard() { + addView("logs-1", "FROM emp | WHERE logs.type == nginx"); + LogicalPlan plan = query("FROM logs-*,-logs-1,logs-*"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp | WHERE logs.type == nginx"))); + } + + public void testExclusionWithDuplicateViewWildcardAndRemainingIndex() { + addView("logs-1", "FROM emp | WHERE logs.type == nginx"); + addIndex("logs-2"); + LogicalPlan plan = query("FROM logs-*,-logs-1,logs-*"); + LogicalPlan rewritten = replaceViews(plan); + assertThat(rewritten, instanceOf(UnionAll.class)); + List subqueries = rewritten.children(); + assertThat(subqueries.size(), equalTo(2)); + assertThat( + subqueries, + containsInAnyOrder(matchesPlan(query("FROM logs-*,logs-*")), matchesPlan(query("FROM emp | WHERE logs.type == nginx"))) + ); + } + + public void testViewBodyWithExclusionCombined() { + addView("safe-logs", "FROM logs*,-logs-secret"); + addIndex("logs-public"); + addIndex("logs-secret"); + LogicalPlan plan = query("FROM safe-logs,logs-secret"); + assertThat(replaceViews(plan), matchesPlan(query("FROM logs*,-logs-secret,logs-secret"))); + } + + public void testExclusionWithNoRemainingIndexMatch() { + addView("logs-nginx", "FROM logs | WHERE logs.type == nginx"); + LogicalPlan plan = query("FROM logs*, -logs-nginx"); + assertThat(replaceViews(plan), matchesPlan(query("FROM logs*"))); + } + + public void testExclusionPreservedForIndexResolution() { + addView("logs1", "FROM logs2"); + addIndex("logs2"); + addIndex("logs3"); + LogicalPlan plan = query("FROM logs*,-logs3"); + assertThat(replaceViews(plan), matchesPlan(query("FROM logs2,logs*,-logs3"))); + } + + public void testExclusionMultipleViews() { + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + addView("view3", "FROM emp3"); + LogicalPlan plan = query("FROM view*, -view1, -view3"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp2"))); + } + + public void testExclusionAllViews() { + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + LogicalPlan plan = query("FROM view*, -view1, -view2"); + assertThat(replaceViews(plan), matchesPlan(query("FROM view*"))); + } + + public void testExclusionKeepingViewWithPipeBody() { + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2 | WHERE emp.age > 30"); + LogicalPlan plan = query("FROM view*, -view1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp2 | WHERE emp.age > 30"))); + } + + public void testExclusionWithWildcardExclusionPattern() { + addView("view_a1", "FROM emp1"); + addView("view_a2", "FROM emp2"); + addView("view_b1", "FROM emp3"); + LogicalPlan plan = query("FROM view_*, -view_a*"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp3"))); + } + + public void testExclusionPreservesNestedViewReference() { + addView("view_inner", "FROM emp1"); + addView("view_outer", "FROM view_inner"); + LogicalPlan plan = query("FROM view_*, -view_inner"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1"))); + } + + public void testExclusionWithMultiplePipeBodies() { + addView("view1", "FROM emp1 | WHERE emp.age > 30"); + addView("view2", "FROM emp2 | WHERE emp.age < 40"); + addView("view3", "FROM emp3 | WHERE emp.salary > 50000"); + LogicalPlan plan = query("FROM view*, -view2"); + LogicalPlan rewritten = replaceViews(plan); + assertThat(rewritten, instanceOf(UnionAll.class)); + List subqueries = rewritten.children(); + assertThat(subqueries.size(), equalTo(2)); + assertThat( + subqueries, + containsInAnyOrder( + matchesPlan(query("FROM emp1 | WHERE emp.age > 30")), + matchesPlan(query("FROM emp3 | WHERE emp.salary > 50000")) + ) + ); + } + + public void testExclusionWithMatchingIndexAndViewExclusion() { + addIndex("viewX"); + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + LogicalPlan plan = query("FROM view*, -view2"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,view*"))); + } + + public void testExclusionAllViewsWithIndex() { + addIndex("viewX"); + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + LogicalPlan plan = query("FROM view*, -view1, -view2"); + assertThat(replaceViews(plan), matchesPlan(query("FROM view*"))); + } + + public void testExclusionNonExistingResource() { + addIndex("viewX"); + addView("view1", "FROM emp1"); + LogicalPlan plan = query("FROM view*, -donotexist"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,view*,-donotexist"))); + } + + public void testFailureSelector() { + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + LogicalPlan plan = query("FROM view*::failures"); + assertThat(replaceViews(plan), matchesPlan(query("FROM view*::failures"))); + } + + public void testConcreteFailureSelector() { + addView("view1", "FROM emp1"); + LogicalPlan plan = query("FROM view1::failures"); + assertThat(replaceViews(plan), matchesPlan(query("FROM view1::failures"))); + } + + public void testDataSelector() { + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + addIndex("view3"); + LogicalPlan plan = query("FROM view*::data"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp2,view*::data"))); + } + + public void testConcreteDataSelector() { + addView("view1", "FROM emp1"); + LogicalPlan plan = query("FROM view1::data"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1"))); + } + + public void testCCSRemoteWildcardNotResolvedAsView() { + addView("view1", "FROM emp1"); + LogicalPlan plan = query("FROM *:view1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM *:view1"))); + } + + public void testCCSExpressionNotResolvedAsView() { + addIndex("remote:view1"); + addView("view1", "FROM emp1"); + LogicalPlan plan = query("FROM remote:view1, view1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,remote:view1"))); + } + + public void testCCSWildcardNotResolvedAsView() { + addView("view1", "FROM emp1"); + LogicalPlan plan = query("FROM remote:view*"); + assertThat(replaceViews(plan), matchesPlan(query("FROM remote:view*"))); } public void testReplaceViewPlans() { @@ -106,8 +300,7 @@ public void testReplaceViewPlans() { addView("view2", "FROM view1 | WHERE emp.age < 40"); addView("view3", "FROM view2 | WHERE emp.salary > 50000"); LogicalPlan plan = query("FROM view3"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp | WHERE emp.age > 30 | WHERE emp.age < 40 | WHERE emp.salary > 50000"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp | WHERE emp.age > 30 | WHERE emp.age < 40 | WHERE emp.salary > 50000"))); } public void testReplaceViews() { @@ -115,8 +308,7 @@ public void testReplaceViews() { addView("view2", "FROM emp2"); addView("view3", "FROM emp3"); LogicalPlan plan = query("FROM view1, view2, view3"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp1, emp2, emp3"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1, emp2, emp3"))); } public void testReplaceViewsPlans() { @@ -124,7 +316,7 @@ public void testReplaceViewsPlans() { addView("view2", "FROM emp2 | WHERE emp.age < 40"); addView("view3", "FROM emp3 | WHERE emp.salary > 50000"); LogicalPlan plan = query("FROM view1, view2, view3"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); // We cannot express the expected plan easily, so we check its structure instead assertThat(rewritten, instanceOf(UnionAll.class)); List subqueries = rewritten.children(); @@ -139,13 +331,33 @@ public void testReplaceViewsPlans() { ); } + public void testReplaceViewsInForkMultipleBranches() { + addView("view1", "FROM emp | WHERE emp.age > 25"); + LogicalPlan plan = query("FROM view1 | FORK (WHERE emp.age < 50) (WHERE emp.age > 35) (STATS count = COUNT(*))"); + Fork fork = (Fork) replaceViews(plan); + List children = fork.children(); + assertThat(children.size(), equalTo(3)); + + assertThat( + as(children.get(0), Eval.class).child(), + equalToIgnoringIds(query("FROM emp | WHERE emp.age > 25 | WHERE emp.age < 50")) + ); + assertThat( + as(children.get(1), Eval.class).child(), + equalToIgnoringIds(query("FROM emp | WHERE emp.age > 25 | WHERE emp.age > 35")) + ); + assertThat( + as(children.get(2), Eval.class).child(), + equalToIgnoringIds(query("FROM emp | WHERE emp.age > 25 | STATS count = COUNT(*)")) + ); + } + public void testReplaceViewsWildcard() { addView("view1", "FROM emp1"); addView("view2", "FROM emp2"); addView("view3", "FROM emp3"); LogicalPlan plan = query("FROM view*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp1, emp2, emp3"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1, emp2, emp3"))); } public void testReplaceViewsWildcardWithIndex() { @@ -154,8 +366,30 @@ public void testReplaceViewsWildcardWithIndex() { addView("view2", "FROM emp2"); addView("view3", "FROM emp3"); LogicalPlan plan = query("FROM view*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM view*, emp1, emp2, emp3"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp2,emp3,view*"))); + } + + public void testMixedViewAndIndexMergedUnresolvedRelation() { + addView("view1", "FROM emp"); + addIndex("index1"); + LogicalPlan plan = query("FROM view1, index1"); + LogicalPlan rewritten = replaceViews(plan); + assertThat(rewritten, instanceOf(UnresolvedRelation.class)); + assertThat(as(rewritten, UnresolvedRelation.class).indexPattern().indexPattern(), equalTo("emp,index1")); + } + + public void testMissingIndexPreservedWhenMixedWithView() { + addView("view1", "FROM emp"); + LogicalPlan plan = query("FROM view1, missing-index"); + LogicalPlan rewritten = replaceViews(plan); + assertThat(as(rewritten, UnresolvedRelation.class).indexPattern().indexPattern(), equalTo("emp,missing-index")); + } + + public void testMissingIndexPreservedWhenMixedWithViewWithPipes() { + addView("view1", "FROM emp | WHERE emp.age > 30"); + LogicalPlan plan = query("FROM view1, missing-index"); + LogicalPlan rewritten = replaceViews(plan); + assertThat(rewritten, instanceOf(UnionAll.class)); } public void testReplaceViewsPlanWildcard() { @@ -163,7 +397,7 @@ public void testReplaceViewsPlanWildcard() { addView("view_2", "FROM emp2 | WHERE emp.age < 40"); addView("view_3", "FROM emp3 | WHERE emp.salary > 50000"); LogicalPlan plan = query("FROM view*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); // We cannot express the expected plan easily, so we check its structure instead assertThat(rewritten, instanceOf(UnionAll.class)); List subqueries = rewritten.children(); @@ -184,7 +418,7 @@ public void testReplaceViewsPlanWildcardWithIndex() { addView("view_2", "FROM emp2 | WHERE emp.age < 40"); addView("view_3", "FROM emp3 | WHERE emp.salary > 50000"); LogicalPlan plan = query("FROM view*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); // We cannot express the expected plan easily, so we check its structure instead assertThat(rewritten, instanceOf(UnionAll.class)); List subqueries = rewritten.children(); @@ -207,8 +441,7 @@ public void testReplaceViewsNestedWildcard() { addView("view_1_2", "FROM view_1, view_2"); addView("view_1_3", "FROM view_1, view_3"); LogicalPlan plan = query("FROM view_1_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp1,emp3,emp1,emp2"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp3,emp1,emp2"))); } public void testReplaceViewsNestedWildcardWithIndex() { @@ -219,8 +452,7 @@ public void testReplaceViewsNestedWildcardWithIndex() { addView("view_1_2", "FROM view_1, view_2"); addView("view_1_3", "FROM view_1, view_3"); LogicalPlan plan = query("FROM view_1_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM view_1_*,emp1,emp3,emp1,emp2"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp3,emp1,emp2,view_1_*"))); } public void testReplaceViewsNestedWildcards() { @@ -234,8 +466,7 @@ public void testReplaceViewsNestedWildcards() { addView("view_3_1", "FROM view_3, view_1"); addView("view_3_2", "FROM view_3, view_2"); LogicalPlan plan = query("FROM view_1_*, view_2_*, view_3_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp1,emp3,emp1,emp2,emp2,emp1,emp2,emp3,emp3,emp1,emp3,emp2"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp3,emp1,emp2,emp2,emp1,emp2,emp3,emp3,emp1,emp3,emp2"))); } public void testReplaceViewsNestedWildcardsWithIndex() { @@ -250,8 +481,7 @@ public void testReplaceViewsNestedWildcardsWithIndex() { addView("view_3_1", "FROM view_3, view_1"); addView("view_3_2", "FROM view_3, view_2"); LogicalPlan plan = query("FROM view_1_*, view_2_*, view_3_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM view_2_*,emp1,emp3,emp1,emp2,emp2,emp1,emp2,emp3,emp3,emp1,emp3,emp2"))); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp3,emp1,emp2,emp2,emp1,emp2,emp3,emp3,emp1,emp3,emp2,view_2_*"))); } public void testReplaceViewsNestedWildcardsWithIndexes() { @@ -268,10 +498,10 @@ public void testReplaceViewsNestedWildcardsWithIndexes() { addView("view_3_1", "FROM view_3, view_1"); addView("view_3_2", "FROM view_3, view_2"); LogicalPlan plan = query("FROM view_1_*, view_2_*, view_3_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); assertThat( rewritten, - matchesPlan(query("FROM view_1_*,view_2_*,view_3_*,emp1,emp3,emp1,emp2,emp2,emp1,emp2,emp3,emp3,emp1,emp3,emp2")) + matchesPlan(query("FROM emp1,emp3,emp1,emp2,emp2,emp1,emp2,emp3,emp3,emp1,emp3,emp2,view_1_*,view_2_*,view_3_*")) ); } @@ -282,7 +512,7 @@ public void testReplaceViewsNestedPlansWildcard() { addView("view_1_2", "FROM view_1, view_2"); addView("view_1_3", "FROM view_1, view_3"); LogicalPlan plan = query("FROM view_1_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); // We cannot express the expected plan easily, so we check its structure instead assertThat(rewritten, instanceOf(UnionAll.class)); List subqueries = rewritten.children(); @@ -312,7 +542,7 @@ public void testReplaceViewsNestedPlansWildcardWithIndex() { addView("view_1_2", "FROM view_1, view_2"); addView("view_1_3", "FROM view_1, view_3"); LogicalPlan plan = query("FROM view_1_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); // We cannot express the expected plan easily, so we check its structure instead assertThat(rewritten, instanceOf(UnionAll.class)); List subqueries = rewritten.children(); @@ -346,7 +576,7 @@ public void testReplaceViewsNestedPlansWildcards() { addView("view_3_1", "FROM view_3, view_1"); addView("view_3_2", "FROM view_3, view_2"); LogicalPlan plan = query("FROM view_1_*, view_2_*, view_3_*"); - LogicalPlan rewritten = viewResolver.replaceViews(plan, this::parse).plan(); + LogicalPlan rewritten = replaceViews(plan); // We cannot express the expected plan easily, so we check its structure instead assertThat(rewritten, instanceOf(UnionAll.class)); List subqueries = rewritten.children(); @@ -382,12 +612,110 @@ public void testViewDepthExceeded() { addView("view11", "FROM view10"); // FROM view11 should fail - Exception e = expectThrows(VerificationException.class, () -> viewResolver.replaceViews(query("FROM view11"), this::parse)); - assertThat(e.getMessage(), startsWith("The maximum allowed view depth of 10 has been exceeded")); - + { + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM view11"))); + assertThat(e.getMessage(), startsWith("The maximum allowed view depth of 10 has been exceeded")); + } // But FROM view10 should work - LogicalPlan rewritten = viewResolver.replaceViews(query("FROM view10"), this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp"))); + { + LogicalPlan rewritten = replaceViews(query("FROM view10")); + assertThat(rewritten, matchesPlan(query("FROM emp"))); + } + } + + public void testCircularViewSelfReference() { + addView("view_a", "FROM view_a"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM view_a"))); + assertThat(e.getMessage(), containsString("circular view reference 'view_a'")); + } + + public void testCircularViewMutualReference() { + addView("view_a", "FROM view_b"); + addView("view_b", "FROM view_a"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM view_a"))); + assertThat(e.getMessage(), containsString("circular view reference 'view_a'")); + assertThat(e.getMessage(), containsString("view_a -> view_b")); + } + + public void testCircularViewChain() { + addView("chain_a", "FROM chain_b"); + addView("chain_b", "FROM chain_c"); + addView("chain_c", "FROM chain_a"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM chain_a"))); + assertThat(e.getMessage(), containsString("circular view reference 'chain_a'")); + assertThat(e.getMessage(), containsString("chain_a -> chain_b -> chain_c")); + } + + public void testCircularViewViaWildcard() { + addView("v_1", "FROM v_*"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM v_*"))); + assertThat(e.getMessage(), containsString("circular view reference 'v_1'")); + } + + public void testCircularViewViaWildcardWithIndex() { + addIndex("v_idx"); + addView("v_1", "FROM v_*"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM v_*"))); + assertThat(e.getMessage(), containsString("circular view reference 'v_1'")); + } + + public void testCircularViewInMultiSource() { + addView("view_a", "FROM emp"); + addView("view_b", "FROM view_c"); + addView("view_c", "FROM view_a"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM view_a, view_b"))); + assertThat(e.getMessage(), containsString("circular view reference 'view_a'")); + } + + public void testCircularViewWithPipes() { + addView("view_a", "FROM view_b | WHERE emp.age > 30"); + addView("view_b", "FROM view_a | WHERE emp.salary > 50000"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM view_a"))); + assertThat(e.getMessage(), containsString("circular view reference 'view_a'")); + assertThat(e.getMessage(), containsString("view_a -> view_b")); + } + + public void testCircularViewInFork() { + addView("view_a", "FROM view_b"); + addView("view_b", "FROM view_a"); + Exception e = expectThrows( + VerificationException.class, + () -> replaceViews(query("FROM view_a | FORK (WHERE emp.age > 30) (WHERE emp.age < 50)")) + ); + assertThat(e.getMessage(), containsString("circular view reference 'view_a'")); + } + + public void testCircularViewExcludedByWildcard() { + addView("v_1", "FROM v_*"); + LogicalPlan plan = query("FROM v_*,-v_1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM v_*"))); + } + + public void testCircularViewExcludedByConcreteExclusion() { + addView("view_a", "FROM view_b"); + addView("view_b", "FROM view_a"); + Exception e = expectThrows(VerificationException.class, () -> replaceViews(query("FROM view_a,-view_b"))); + assertThat(e.getMessage(), containsString("circular view reference 'view_a'")); + } + + public void testCircularViewBodyWithSelfExclusion() { + addView("v_1", "FROM v_*,-v_1"); + LogicalPlan plan = query("FROM v_1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM v_*"))); + } + + public void testCircularViewBodyWithSelfExclusionAndIndex() { + addIndex("v_idx"); + addView("v_1", "FROM v_*,-v_1"); + LogicalPlan plan = query("FROM v_1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM v_*"))); + } + + public void testCircularViewBodyWithSelfExclusionAndOtherView() { + addView("v_1", "FROM v_*,-v_1"); + addView("v_2", "FROM emp"); + LogicalPlan plan = query("FROM v_1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp"))); } public void testModifiedViewDepth() { @@ -396,22 +724,26 @@ public void testModifiedViewDepth() { Settings.builder().put(ViewResolver.MAX_VIEW_DEPTH_SETTING.getKey(), 1).build() ) ) { + customViewService.addIndex(projectId, "emp"); addView("view1", "FROM emp", customViewService); addView("view2", "FROM view1", customViewService); addView("view3", "FROM view2", customViewService); InMemoryViewResolver customViewResolver = customViewService.getViewResolver(); - - // FROM view2 should fail - Exception e = expectThrows( - VerificationException.class, - () -> customViewResolver.replaceViews(query("FROM view2"), this::parse) - ); - assertThat(e.getMessage(), startsWith("The maximum allowed view depth of 1 has been exceeded")); - + { + PlainActionFuture future = new PlainActionFuture<>(); + customViewResolver.replaceViews(query("FROM view2"), this::parse, future); + // FROM view2 should fail + Exception e = expectThrows(VerificationException.class, future::actionGet); + assertThat(e.getMessage(), startsWith("The maximum allowed view depth of 1 has been exceeded")); + } // But FROM view1 should work - LogicalPlan rewritten = customViewResolver.replaceViews(query("FROM view1"), this::parse).plan(); - assertThat(rewritten, matchesPlan(query("FROM emp"))); + { + PlainActionFuture future = new PlainActionFuture<>(); + customViewResolver.replaceViews(query("FROM view1"), this::parse, future); + LogicalPlan rewritten = future.actionGet().plan(); + assertThat(rewritten, matchesPlan(query("FROM emp"))); + } } catch (Exception e) { throw new AssertionError("unexpected exception", e); } @@ -470,6 +802,89 @@ public void testModifiedViewLength() { } } + public void testViewWithDateMathInBody() { + addDateMathIndex("logs-"); + addView("view1", "FROM "); + LogicalPlan plan = query("FROM view1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM "))); + } + + public void testNestedViewWithDateMathInBody() { + addDateMathIndex("logs-"); + addView("view1", "FROM "); + addView("view2", "FROM view1"); + LogicalPlan plan = query("FROM view2"); + assertThat(replaceViews(plan), matchesPlan(query("FROM "))); + } + + public void testViewWithDateMathAndPipeInBody() { + addDateMathIndex("logs-"); + addView("view1", "FROM | WHERE log.level == \"error\""); + LogicalPlan plan = query("FROM view1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM | WHERE log.level == \"error\""))); + } + + public void testDateMathResolvesToViewName() { + var date = LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC); + var resolvedName = DateTimeFormatter.ofPattern("'view-'yyyy.MM.dd", Locale.ROOT).format(date); + addView(resolvedName, "FROM emp"); + try { + LogicalPlan plan = query("FROM "); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp"))); + } catch (AssertionError e) { + assumeTrue("Date must stay the same during the test", Objects.equals(date, LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC))); + throw e; + } + } + + public void testDateMathAlongsideConcreteView() { + var date = LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC); + var resolvedName = DateTimeFormatter.ofPattern("'logs-'yyyy.MM.dd", Locale.ROOT).format(date); + addIndex(resolvedName); + addView("view1", "FROM emp"); + try { + LogicalPlan plan = query("FROM , view1"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp,"))); + } catch (AssertionError e) { + assumeTrue("Date must stay the same during the test", Objects.equals(date, LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC))); + throw e; + } + } + + public void testDateMathAlongsideViewWildcard() { + var date = LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC); + var resolvedName = DateTimeFormatter.ofPattern("'logs-'yyyy.MM.dd", Locale.ROOT).format(date); + addIndex(resolvedName); + addView("view1", "FROM emp1"); + addView("view2", "FROM emp2"); + try { + LogicalPlan plan = query("FROM , view*"); + assertThat(replaceViews(plan), matchesPlan(query("FROM emp1,emp2,"))); + } catch (AssertionError e) { + assumeTrue("Date must stay the same during the test", Objects.equals(date, LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC))); + throw e; + } + } + + public void testDateMathAlongsideViewWithPipeBody() { + var date = LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC); + var resolvedName = DateTimeFormatter.ofPattern("'logs-'yyyy.MM.dd", Locale.ROOT).format(date); + addIndex(resolvedName); + addView("view1", "FROM emp | WHERE emp.age > 30"); + try { + LogicalPlan plan = query("FROM , view1"); + LogicalPlan rewritten = replaceViews(plan); + assertThat(rewritten, instanceOf(UnionAll.class)); + List subqueries = rewritten.children(); + assertThat(subqueries.size(), equalTo(2)); + assertThat(subqueries.getFirst(), matchesPlan(query("FROM "))); + assertThat(subqueries.get(1), matchesPlan(query("FROM emp | WHERE emp.age > 30"))); + } catch (AssertionError e) { + assumeTrue("Date must stay the same during the test", Objects.equals(date, LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC))); + throw e; + } + } + public void testSerializationSubqueryWithSourceFromViewQuery() { // This test verifies that view sources are correctly tagged with their view name // and that the Configuration contains the view queries, allowing proper deserialization. @@ -526,8 +941,20 @@ public void testSerializationSubqueryWithSourceFromViewQuery() { ); } + private LogicalPlan replaceViews(LogicalPlan plan) { + PlainActionFuture future = new PlainActionFuture<>(); + viewResolver.replaceViews(plan, this::parse, future); + return future.actionGet().plan(); + } + private void addIndex(String name) { - viewResolver.addIndex(name); + viewService.addIndex(projectId, name); + } + + private void addDateMathIndex(String prefix) { + addIndex( + prefix + LocalDate.ofInstant(Instant.now(), ZoneOffset.UTC).format(DateTimeFormatter.ofPattern("yyyy.MM.dd", Locale.ROOT)) + ); } private void addView(String name, String query) { @@ -635,4 +1062,12 @@ private static void generateCombinations( current.remove(current.size() - 1); } } + + protected LogicalPlan query(String e) { + return query(e, new QueryParams()); + } + + LogicalPlan query(String e, QueryParams params) { + return parser.parseQuery(e, params); + } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/ViewResolutionServiceTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/ViewResolutionServiceTests.java new file mode 100644 index 0000000000000..50ab8d5cfc29e --- /dev/null +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/view/ViewResolutionServiceTests.java @@ -0,0 +1,57 @@ +/* + * 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.view; + +import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.ProjectId; +import org.elasticsearch.cluster.metadata.ProjectMetadata; +import org.elasticsearch.cluster.project.TestProjectResolvers; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.indices.EmptySystemIndices; +import org.elasticsearch.test.ESTestCase; + +import static org.elasticsearch.action.support.IndicesOptions.ConcreteTargetOptions.ERROR_WHEN_UNAVAILABLE_TARGETS; + +public class ViewResolutionServiceTests extends ESTestCase { + + public void testResolveMissing() { + ViewResolutionService service = newService(); + ClusterState clusterState = emptyClusterState(); + assertThrows( + IndexNotFoundException.class, + () -> service.resolveViews( + clusterState.projectState(ProjectId.DEFAULT), + new String[] { "missing" }, + IndicesOptions.builder() + .wildcardOptions(IndicesOptions.WildcardOptions.builder().resolveViews(true)) + .concreteTargetOptions(ERROR_WHEN_UNAVAILABLE_TARGETS) + .build(), + null + ) + ); + } + + private static ViewResolutionService newService() { + IndexNameExpressionResolver resolver = new IndexNameExpressionResolver( + new ThreadContext(Settings.EMPTY), + EmptySystemIndices.INSTANCE, + TestProjectResolvers.DEFAULT_PROJECT_ONLY + ); + return new ViewResolutionService(resolver); + } + + private static ClusterState emptyClusterState() { + return ClusterState.builder(ClusterName.DEFAULT).putProjectMetadata(ProjectMetadata.builder(ProjectId.DEFAULT).build()).build(); + } + +} diff --git a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java index 919b49191e49d..b35c8179df2cd 100644 --- a/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java +++ b/x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java @@ -593,6 +593,7 @@ public class Constants { "indices:data/read/esql/async/get", "indices:data/read/esql/async/stop", "indices:data/read/esql/resolve_fields", + "indices:data/read/esql/resolve_views", "indices:data/read/esql/search_shards", "indices:data/read/explain", "indices:data/read/field_caps", diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index 75b5f1d81a33f..65f34e7a1bb4b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -430,8 +430,8 @@ ResolvedIndices resolveIndicesAndAliases( } var resolved = resolvedExpressionsBuilder.build(); - if (crossProjectModeDecider.crossProjectEnabled()) { - setResolvedIndexExpressionsIfUnset(replaceable, resolved); + if (shouldSetResolvedIndexExpressions(replaceable, resolved)) { + replaceable.setResolvedIndexExpressions(resolved); } resolvedIndicesBuilder.addLocal(resolved.getLocalIndicesList()); resolvedIndicesBuilder.addRemote(resolved.getRemoteIndicesList()); @@ -449,8 +449,9 @@ ResolvedIndices resolveIndicesAndAliases( replaceable.getProjectRouting() ); } - if (crossProjectModeDecider.crossProjectEnabled()) { - setResolvedIndexExpressionsIfUnset(replaceable, ResolvedIndexExpressions.builder().build()); + var resolved = ResolvedIndexExpressions.builder().build(); + if (shouldSetResolvedIndexExpressions(replaceable, resolved)) { + replaceable.setResolvedIndexExpressions(resolved); } } @@ -481,7 +482,9 @@ ResolvedIndices resolveIndicesAndAliases( indicesRequest.includeDataStreams(), replaceable.getProjectRouting() ); - setResolvedIndexExpressionsIfUnset(replaceable, resolved); + if (shouldSetResolvedIndexExpressions(replaceable, resolved)) { + replaceable.setResolvedIndexExpressions(resolved); + } resolvedIndicesBuilder.addLocal(resolved.getLocalIndicesList()); resolvedIndicesBuilder.addRemote(resolved.getRemoteIndicesList()); } else { @@ -499,11 +502,8 @@ ResolvedIndices resolveIndicesAndAliases( authorizedIndices::check, indicesRequest.includeDataStreams() ); - // only store resolved expressions if configured, to avoid unnecessary memory usage - // once we've migrated from `indices()` to using resolved expressions holistically, - // we will always store them - if (crossProjectModeDecider.crossProjectEnabled()) { - setResolvedIndexExpressionsIfUnset(replaceable, resolved); + if (shouldSetResolvedIndexExpressions(replaceable, resolved)) { + replaceable.setResolvedIndexExpressions(resolved); } resolvedIndicesBuilder.addLocal(resolved.getLocalIndicesList()); resolvedIndicesBuilder.addRemote(split.getRemote()); @@ -576,10 +576,15 @@ ResolvedIndices resolveIndicesAndAliases( return resolvedIndicesBuilder.build(); } - private static void setResolvedIndexExpressionsIfUnset(IndicesRequest.Replaceable replaceable, ResolvedIndexExpressions resolved) { - if (replaceable.getResolvedIndexExpressions() == null) { - replaceable.setResolvedIndexExpressions(resolved); - } else { + private boolean shouldSetResolvedIndexExpressions(IndicesRequest.Replaceable replaceable, ResolvedIndexExpressions resolved) { + // Only store resolved expressions if cross-project mode or if views should be resolved, to avoid unnecessary memory usage. Once + // we've migrated from `indices()` to using resolved expressions holistically, we will always store them + if (crossProjectModeDecider.crossProjectEnabled() == false + && replaceable.indicesOptions().wildcardOptions().resolveViews() == false) { + return false; + } + + if (replaceable.getResolvedIndexExpressions() != null) { // see https://github.com/elastic/elasticsearch/issues/135799 and ES-4376 String message = "resolved index expressions are already set to [" + replaceable.getResolvedIndexExpressions() @@ -598,7 +603,9 @@ private static void setResolvedIndexExpressionsIfUnset(IndicesRequest.Replaceabl // As a result, the resolved indices from the second resolution must be identical (most likely) or a subset of the // resolved indices from the first resolution if the user's role changes in between the two authorizations. assert replaceable.getResolvedIndexExpressions().getLocalIndicesList().containsAll(resolved.getLocalIndicesList()) : message; + return false; } + return true; } /**