Skip to content

[Failure Store] Fix resolved alias retrieval for failure indices (#127458) #127498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2025
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
[Failure Store] Fix resolved alias retrieval for failure indices (#12…
…7458)

While expanding our tests in #126891, we discovered that there was a difference in behaviour when an alias was used during search.

Expected behaviour

When a user uses an alias to access data, we pass this alias also to the node requests because that ensures that the user's authorisation will be evaluated based on the same premise.

Observed behaviour

When a user would use the alias and the ::failures selector, we noticed that the request was resolved to the failure indices and the alias was not used further which sometimes resulted in an unauthorised error. The reason was that when a node different than the coordinating node would try to evaluate if the user has permissions, they would evaluate that against the failure indices themselves and not the alias with the ::failures selector.

Solution
In this PR we ensure that a resolved alias is retrieved for failure indices too.

The indexAliases method is used in two ways (in production code):

To retrieve all the resolved aliases that match an index
To retrieve the filtered aliases that match the index (when there is no unfiltered reference to this index)
Because failure indices are not supported by filtered aliases, the code would return null when a failure index was encountered. That works well for the second case and not for the first.

In order to address that we moved the check for the failure index to the data stream alias predicate and we let the code match failure indices against resolved failure expressions and backing indices against resolved data expressions.

Furthermore, we added methods capturing these two common cases so the users of these methods for not need to know the details of how to call them.

(cherry picked from commit f209db6)
  • Loading branch information
gmarouli committed Apr 29, 2025
commit aae0c91d3f5fc40b72a24fc8afd1a7abc6888236
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.elasticsearch.cluster.routing.ShardIterator;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.core.Predicates;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.injection.guice.Inject;
Expand Down Expand Up @@ -89,14 +88,7 @@ protected void masterOperation(
Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
for (String index : concreteIndices) {
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, indicesAndAliases);
final String[] aliases = indexNameExpressionResolver.indexAliases(
clusterState,
index,
Predicates.always(),
Predicates.always(),
true,
indicesAndAliases
);
final String[] aliases = indexNameExpressionResolver.allIndexAliases(clusterState, index, indicesAndAliases);
indicesAndFilters.put(index, AliasFilter.of(aliasFilter.getQueryBuilder(), aliases));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ private XContentBuilder indicesToXContent(XContentBuilder builder, List<Index> i
}
builder.field(MANAGED_BY.getPreferredName(), indexProperties.managedBy.displayValue);
}

builder.endObject();
}
builder.endArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.concurrent.CountDown;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.core.Predicates;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
Expand Down Expand Up @@ -221,14 +220,7 @@ private Map<String, OriginalIndices> buildPerIndexOriginalIndices(
blocks.indexBlockedRaiseException(ClusterBlockLevel.READ, index);
}

String[] aliases = indexNameExpressionResolver.indexAliases(
clusterState,
index,
Predicates.always(),
Predicates.always(),
true,
indicesAndAliases
);
String[] aliases = indexNameExpressionResolver.allIndexAliases(clusterState, index, indicesAndAliases);
String[] finalIndices = Strings.EMPTY_ARRAY;
if (aliases == null
|| aliases.length == 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.Set;
import java.util.SortedMap;
import java.util.function.BiFunction;
import java.util.function.BiPredicate;
import java.util.function.Function;
import java.util.function.LongSupplier;
import java.util.function.Predicate;
Expand All @@ -79,6 +80,13 @@ public class IndexNameExpressionResolver {
public static final String EXCLUDED_DATA_STREAMS_KEY = "es.excluded_ds";
public static final IndexVersion SYSTEM_INDEX_ENFORCEMENT_INDEX_VERSION = IndexVersions.V_8_0_0;

private static final BiPredicate<DataStreamAlias, Boolean> ALL_DATA_STREAM_ALIASES = (ignoredAlias, ignoredIsData) -> true;
// Alias filters are not applied against indices in an abstraction's failure component.
// They do not match the mapping of the data stream nor are the documents mapped for searching.
private static final BiPredicate<DataStreamAlias, Boolean> ONLY_FILTERING_DATA_STREAM_ALIASES = (
dataStreamAlias,
isData) -> dataStreamAlias.filteringRequired() && isData;

private final ThreadContext threadContext;
private final SystemIndices systemIndices;

Expand Down Expand Up @@ -918,7 +926,16 @@ public Set<ResolvedExpression> resolveExpressions(
* <b>NOTE</b>: The provided expressions must have been resolved already via {@link #resolveExpressionsToResources(Context, String...)}.
*/
public String[] filteringAliases(ClusterState state, String index, Set<ResolvedExpression> resolvedExpressions) {
return indexAliases(state, index, AliasMetadata::filteringRequired, DataStreamAlias::filteringRequired, false, resolvedExpressions);
return indexAliases(state, index, AliasMetadata::filteringRequired, ONLY_FILTERING_DATA_STREAM_ALIASES, false, resolvedExpressions);
}

/**
* Iterates through the list of indices and selects the effective list of all aliases for the
* given index. Aliases are returned even if the index is included in the resolved expressions.
* <b>NOTE</b>: The provided expressions must have been resolved already via {@link #resolveExpressions}.
*/
public String[] allIndexAliases(ClusterState state, String index, Set<ResolvedExpression> resolvedExpressions) {
return indexAliases(state, index, Predicates.always(), ALL_DATA_STREAM_ALIASES, true, resolvedExpressions);
}

/**
Expand All @@ -942,7 +959,7 @@ public String[] indexAliases(
ClusterState state,
String index,
Predicate<AliasMetadata> requiredAlias,
Predicate<DataStreamAlias> requiredDataStreamAlias,
BiPredicate<DataStreamAlias, Boolean> requiredDataStreamAlias,
boolean skipIdentity,
Set<ResolvedExpression> resolvedExpressions
) {
Expand All @@ -969,13 +986,8 @@ public String[] indexAliases(
IndexAbstraction ia = state.metadata().getIndicesLookup().get(index);
DataStream dataStream = ia.getParentDataStream();
if (dataStream != null) {
if (dataStream.getFailureComponent().containsIndex(index)) {
// Alias filters are not applied against indices in an abstraction's failure component.
// They do not match the mapping of the data stream nor are the documents mapped for searching.
return null;
}

if (skipIdentity == false && resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStream.getName())) {
boolean isData = dataStream.isFailureStoreIndex(index) == false;
if (skipIdentity == false && resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStream.getName(), isData)) {
// skip the filters when the request targets the data stream name + selector directly
return null;
}
Expand All @@ -984,12 +996,14 @@ public String[] indexAliases(
if (iterateIndexAliases(dataStreamAliases.size(), resolvedExpressions.size())) {
aliasesForDataStream = dataStreamAliases.values()
.stream()
.filter(dataStreamAlias -> resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStreamAlias.getName()))
.filter(
dataStreamAlias -> resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStreamAlias.getName(), isData)
)
.filter(dataStreamAlias -> dataStreamAlias.getDataStreams().contains(dataStream.getName()))
.toList();
} else {
aliasesForDataStream = resolvedExpressions.stream()
.filter(expression -> expression.selector() == null || expression.selector().shouldIncludeData())
.filter(expression -> (expression.selector() == null || expression.selector().shouldIncludeData()) == isData)
.map(ResolvedExpression::resource)
.map(dataStreamAliases::get)
.filter(dataStreamAlias -> dataStreamAlias != null && dataStreamAlias.getDataStreams().contains(dataStream.getName()))
Expand All @@ -998,11 +1012,14 @@ public String[] indexAliases(

List<String> requiredAliases = null;
for (DataStreamAlias dataStreamAlias : aliasesForDataStream) {
if (requiredDataStreamAlias.test(dataStreamAlias)) {
if (requiredDataStreamAlias.test(dataStreamAlias, isData)) {
if (requiredAliases == null) {
requiredAliases = new ArrayList<>(aliasesForDataStream.size());
}
requiredAliases.add(dataStreamAlias.getName());
String alias = isData
? dataStreamAlias.getName()
: combineSelector(dataStreamAlias.getName(), IndexComponentSelector.FAILURES);
requiredAliases.add(alias);
} else {
// we have a non-required alias for this data stream so no need to check further
return null;
Expand All @@ -1020,7 +1037,7 @@ public String[] indexAliases(
aliasCandidates = indexAliases.values()
.stream()
// Indices can only be referenced with a data selector, or a null selector if selectors are disabled
.filter(aliasMetadata -> resolvedExpressionsContainsAbstraction(resolvedExpressions, aliasMetadata.alias()))
.filter(aliasMetadata -> resolvedExpressionsContainsAbstraction(resolvedExpressions, aliasMetadata.alias(), true))
.toArray(AliasMetadata[]::new);
} else {
// faster to iterate resolvedExpressions
Expand Down Expand Up @@ -1051,9 +1068,16 @@ public String[] indexAliases(
}
}

private static boolean resolvedExpressionsContainsAbstraction(Set<ResolvedExpression> resolvedExpressions, String abstractionName) {
return resolvedExpressions.contains(new ResolvedExpression(abstractionName))
|| resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.DATA));
private static boolean resolvedExpressionsContainsAbstraction(
Set<ResolvedExpression> resolvedExpressions,
String abstractionName,
boolean isData
) {
if (isData) {
return resolvedExpressions.contains(new ResolvedExpression(abstractionName))
|| resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.DATA));
}
return resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.FAILURES));
}

/**
Expand Down
Loading