Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public List<RestHandler> getRestHandlers(
Predicate<NodeFeature> clusterSupportsFeature
) {
List<RestHandler> handlers = new ArrayList<>();
handlers.add(new PainlessExecuteAction.RestAction());
handlers.add(new PainlessExecuteAction.RestAction(settings));
handlers.add(new PainlessContextAction.RestAction());
return handlers;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.network.NetworkAddress;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.XContentHelper;
Expand Down Expand Up @@ -94,6 +95,7 @@
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.script.StringFieldScript;
import org.elasticsearch.search.crossproject.CrossProjectModeDecider;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -351,6 +353,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
private final Script script;
private final ScriptContext<?> context;
private final ContextSetup contextSetup;
private transient IndicesOptions indicesOptions;

static Request parse(XContentParser parser) throws IOException {
return PARSER.parse(parser, null);
Expand Down Expand Up @@ -390,6 +393,14 @@ public ContextSetup getContextSetup() {
return contextSetup;
}

@Override
public void markOriginOnly() {
assert contextSetup != null
: "Painless/execute request without context setup can't have index, this method shouldn't be called";
// strip off cluster alias from the index in this request
index(contextSetup.getIndex());
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
Expand All @@ -407,6 +418,20 @@ public ActionRequestValidationException validate() {
return validationException;
}

public void enableCrossProjectMode() {
this.indicesOptions = IndicesOptions.builder(super.indicesOptions())
.crossProjectModeOptions(new IndicesOptions.CrossProjectModeOptions(true))
.build();
}

@Override
public IndicesOptions indicesOptions() {
if (indicesOptions == null) {
return super.indicesOptions();
}
return indicesOptions;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
Expand Down Expand Up @@ -532,7 +557,11 @@ public TransportAction(

@Override
protected void doExecute(Task task, Request request, ActionListener<Response> listener) {
if (request.getContextSetup() == null || request.getContextSetup().getClusterAlias() == null) {
// By this point index resolution has completed, and we should not try to resolve indices for child requests
// to avoid the second attempt of project authorization in CPS
request.indicesOptions = null;

if (isLocalIndex(request)) {
super.doExecute(task, request, listener);
} else {
// forward to remote cluster after stripping off the clusterAlias from the index expression
Expand All @@ -547,10 +576,19 @@ protected void doExecute(Task task, Request request, ActionListener<Response> li
}
}

private boolean isLocalIndex(Request request) {
if (request.getContextSetup() == null) {
return true;
}
String index = request.index();
return index == null || RemoteClusterAware.isRemoteIndexName(index) == false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do you handle _origin:myindex? Will it ("_origin") have been stripped off before this is called? Otherwise RemoteClusterAware.isRemoteIndexName("_origin:myindex) would return true right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow is:

  1. In IndicesAndAliasesResolver, we check if the request is SingleIndexNoWildcards and indicesOptions().resolveCrossProjectIndexExpression()
  2. We detect if the index expression is local (no cluster prefix or has _origin or has origin alias prefix)
  3. If local, we call setLocal(true) which strips off the cluster alias from the index in the request

So the cluster alias stripping happens during the security action filter call. This differs from my original approach, but I think it's the best way since we have the origin alias available there but not when execute is called on the transport action.

}

// Visible for testing
static void removeClusterAliasFromIndexExpression(Request request) {
if (request.index() != null) {
String[] split = RemoteClusterAware.splitIndexName(request.index());
String index = request.index();
if (index != null) {
String[] split = RemoteClusterAware.splitIndexName(index);
if (split[0] != null) {
/*
* if the cluster alias is null and the index field has a clusterAlias (clusterAlias:index notation)
Expand Down Expand Up @@ -854,6 +892,11 @@ private static Response prepareRamIndex(

@ServerlessScope(Scope.PUBLIC)
public static class RestAction extends BaseRestHandler {
private final CrossProjectModeDecider crossProjectModeDecider;

public RestAction(Settings settings) {
this.crossProjectModeDecider = new CrossProjectModeDecider(settings);
}

@Override
public List<Route> routes() {
Expand All @@ -868,6 +911,10 @@ public String getName() {
@Override
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
final Request request = Request.parse(restRequest.contentOrSourceParamParser());
if (crossProjectModeDecider.crossProjectEnabled()) {
request.enableCrossProjectMode();
}

return channel -> client.executeLocally(INSTANCE, request, new RestToXContentListener<>(channel));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,12 @@ public void testRemoveClusterAliasFromIndexExpression() {
}
}

public void testMarkOriginOnly() {
PainlessExecuteAction.Request request = createRequest("p1:blogs");
request.markOriginOnly();
assertThat(request.index(), equalTo("blogs"));
}

private PainlessExecuteAction.Request createRequest(String indexExpression) {
return new PainlessExecuteAction.Request(
new Script("100.0 / 1000.0"),
Expand Down
14 changes: 14 additions & 0 deletions server/src/main/java/org/elasticsearch/action/IndicesRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,20 @@ interface SingleIndexNoWildcards extends IndicesRequest, CrossProjectCandidate {
default boolean allowsRemoteIndices() {
return true;
}

/**
* Determines whether the request type allows cross-project processing. Cross-project processing entails cross-project search
* index resolution and error handling. Note: this method only determines in the request _supports_ cross-project.
* Whether cross-project processing is actually performed is determined by {@link IndicesOptions}.
*/
default boolean allowsCrossProject() {
return true;
}
Comment on lines +122 to +129
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We making change so that UIAM authentication and authorized project resolution are separated from CPS index resolution so that they can be available for more request types. Please refer to the relevant change for this file here. Thanks!


/**
* Marks request local. Local requests should be processed on the same cluster, even if they have cluster-alias prefix.
*/
void markOriginOnly();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ private AsyncSupplier<ResolvedIndices> makeResolvedIndicesAsyncSupplier(
var resolvedIndices = indicesAndAliasesResolver.resolvePITIndices(searchRequest);
return SubscribableListener.newSucceeded(resolvedIndices);
}
final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request);
final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request, targetProjects);
if (resolvedIndices != null) {
return SubscribableListener.newSucceeded(resolvedIndices);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.search.crossproject.CrossProjectIndexExpressionsRewriter;
import org.elasticsearch.search.crossproject.CrossProjectModeDecider;
import org.elasticsearch.search.crossproject.ProjectRoutingInfo;
import org.elasticsearch.search.crossproject.ProjectRoutingResolver;
import org.elasticsearch.search.crossproject.TargetProjects;
import org.elasticsearch.transport.LinkedProjectConfig;
Expand All @@ -57,6 +58,7 @@
import java.util.SortedMap;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.function.BiPredicate;
import java.util.stream.Collectors;

import static org.elasticsearch.search.crossproject.CrossProjectIndexResolutionValidator.indicesOptionsForCrossProjectFanout;
import static org.elasticsearch.xpack.core.security.authz.IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER;
Expand Down Expand Up @@ -152,7 +154,7 @@ ResolvedIndices resolve(
* @return The {@link ResolvedIndices} or null if wildcard expansion must be performed.
*/
@Nullable
ResolvedIndices tryResolveWithoutWildcards(String action, TransportRequest transportRequest) {
ResolvedIndices tryResolveWithoutWildcards(String action, TransportRequest transportRequest, TargetProjects authorizedProjects) {
// We only take care of IndicesRequest
if (false == transportRequest instanceof IndicesRequest) {
return null;
Expand All @@ -162,7 +164,7 @@ ResolvedIndices tryResolveWithoutWildcards(String action, TransportRequest trans
return null;
}
// It's safe to cast IndicesRequest since the above test guarantees it
return resolveIndicesAndAliasesWithoutWildcards(action, indicesRequest);
return resolveIndicesAndAliasesWithoutWildcards(action, indicesRequest, authorizedProjects);
}

boolean resolvesCrossProject(TransportRequest request) {
Expand All @@ -183,6 +185,14 @@ private static boolean requiresWildcardExpansion(IndicesRequest indicesRequest)
}

ResolvedIndices resolveIndicesAndAliasesWithoutWildcards(String action, IndicesRequest indicesRequest) {
return resolveIndicesAndAliasesWithoutWildcards(action, indicesRequest, TargetProjects.LOCAL_ONLY_FOR_CPS_DISABLED);
}

ResolvedIndices resolveIndicesAndAliasesWithoutWildcards(
String action,
IndicesRequest indicesRequest,
TargetProjects authorizedProjects
) {
assert false == requiresWildcardExpansion(indicesRequest) : "request must not require wildcard expansion";
final String[] indices = indicesRequest.indices();
if (indices == null || indices.length == 0) {
Expand All @@ -205,17 +215,17 @@ ResolvedIndices resolveIndicesAndAliasesWithoutWildcards(String action, IndicesR
}

final ResolvedIndices split;
if (indicesRequest instanceof IndicesRequest.SingleIndexNoWildcards single && single.allowsRemoteIndices()) {
split = remoteClusterResolver.splitLocalAndRemoteIndexNames(indicesRequest.indices());
// all indices can come back empty when the remote index expression included a cluster alias with a wildcard
// and no remote clusters are configured that match it
if (split.getLocal().isEmpty() && split.getRemote().isEmpty()) {
for (String indexExpression : indices) {
String[] clusterAndIndex = RemoteClusterAware.splitIndexName(indexExpression);
if (clusterAndIndex[0] != null && clusterAndIndex[0].contains("*")) {
throw new NoSuchRemoteClusterException(clusterAndIndex[0]);
}
if (indicesRequest instanceof IndicesRequest.SingleIndexNoWildcards single
&& (single.allowsRemoteIndices() || single.allowsCrossProject())) {
assert indices.length == 1 : "SingleIndexNoWildcards request must have exactly one index";

if (crossProjectModeDecider.resolvesCrossProject(single)) {
split = remoteClusterResolver.determineLocalOrRemoteIndexCrossProject(authorizedProjects, indices[0]);
if (split.getLocal().isEmpty() == false) {
single.markOriginOnly();
}
} else {
split = remoteClusterResolver.determineLocalOrRemoteIndex(indices[0]);
}
} else {
split = new ResolvedIndices(Arrays.asList(indicesRequest.indices()), List.of());
Expand Down Expand Up @@ -501,7 +511,7 @@ ResolvedIndices resolveIndicesAndAliases(
// That's why an assertion error is triggered here so that we can catch the erroneous usage in testing.
// But we still delegate in production to avoid our (potential) programing error becoming an end-user problem.
assert false : "Request [" + indicesRequest + "] is not a replaceable request, but should be.";
return resolveIndicesAndAliasesWithoutWildcards(action, indicesRequest);
return resolveIndicesAndAliasesWithoutWildcards(action, indicesRequest, authorizedProjects);
}

if (indicesRequest instanceof AliasesRequest aliasesRequest) {
Expand Down Expand Up @@ -714,5 +724,40 @@ ResolvedIndices splitLocalAndRemoteIndexNames(String... indices) {
.toList();
return new ResolvedIndices(local == null ? List.of() : local, remote);
}

ResolvedIndices determineLocalOrRemoteIndex(String indexExpression) {
return determineLocalOrRemoteIndex(indexExpression, Collections.emptySet(), clusters);
}

ResolvedIndices determineLocalOrRemoteIndexCrossProject(TargetProjects targetProjects, String indexExpression) {
assert targetProjects.linkedProjects() != null : "CPS should be enabled to call this method";

Set<String> linkedAliases = targetProjects.linkedProjects()
.stream()
.map(ProjectRoutingInfo::projectAlias)
.collect(Collectors.toSet());

ProjectRoutingInfo originRoutingInfo = targetProjects.originProject();
Set<String> originAliases;
if (originRoutingInfo != null) {
originAliases = Set.of(originRoutingInfo.projectAlias(), ProjectRoutingResolver.ORIGIN);
} else {
originAliases = Collections.emptySet();
}
return determineLocalOrRemoteIndex(indexExpression, originAliases, linkedAliases);
}

private ResolvedIndices determineLocalOrRemoteIndex(String indexExpression, Set<String> originAliases, Set<String> remoteAliases) {
String[] split = RemoteClusterAware.splitIndexName(indexExpression);
String clusterAlias = split[0];
if (clusterAlias == null || originAliases.contains(clusterAlias)) {
return new ResolvedIndices(List.of(split[1]), List.of());
} else {
if (remoteAliases.contains(clusterAlias) == false) {
throw new NoSuchRemoteClusterException(clusterAlias);
}
return new ResolvedIndices(List.of(), List.of(indexExpression));
}
}
}
}
Loading