Skip to content

Commit 182b9b4

Browse files
gmarouligmarouli
authored
[Failure Store] Fix resolved alias retrieval for failure indices (#127458) (#127498)
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)
1 parent d5856f5 commit 182b9b4

File tree

6 files changed

+446
-106
lines changed

6 files changed

+446
-106
lines changed

‎server/src/main/java/org/elasticsearch/action/admin/cluster/shards/TransportClusterSearchShardsAction.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.elasticsearch.cluster.routing.ShardIterator;
2323
import org.elasticsearch.cluster.routing.ShardRouting;
2424
import org.elasticsearch.cluster.service.ClusterService;
25-
import org.elasticsearch.core.Predicates;
2625
import org.elasticsearch.index.shard.ShardId;
2726
import org.elasticsearch.indices.IndicesService;
2827
import org.elasticsearch.injection.guice.Inject;
@@ -89,14 +88,7 @@ protected void masterOperation(
8988
Set<ResolvedExpression> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
9089
for (String index : concreteIndices) {
9190
final AliasFilter aliasFilter = indicesService.buildAliasFilter(clusterState, index, indicesAndAliases);
92-
final String[] aliases = indexNameExpressionResolver.indexAliases(
93-
clusterState,
94-
index,
95-
Predicates.always(),
96-
Predicates.always(),
97-
true,
98-
indicesAndAliases
99-
);
91+
final String[] aliases = indexNameExpressionResolver.allIndexAliases(clusterState, index, indicesAndAliases);
10092
indicesAndFilters.put(index, AliasFilter.of(aliasFilter.getQueryBuilder(), aliases));
10193
}
10294

‎server/src/main/java/org/elasticsearch/action/datastreams/GetDataStreamAction.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,7 @@ private XContentBuilder indicesToXContent(XContentBuilder builder, List<Index> i
455455
}
456456
}
457457
}
458+
458459
builder.endObject();
459460
}
460461
builder.endArray();

‎server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import org.elasticsearch.common.util.Maps;
6060
import org.elasticsearch.common.util.concurrent.CountDown;
6161
import org.elasticsearch.common.util.concurrent.EsExecutors;
62-
import org.elasticsearch.core.Predicates;
6362
import org.elasticsearch.core.TimeValue;
6463
import org.elasticsearch.index.Index;
6564
import org.elasticsearch.index.IndexNotFoundException;
@@ -221,14 +220,7 @@ private Map<String, OriginalIndices> buildPerIndexOriginalIndices(
221220
blocks.indexBlockedRaiseException(ClusterBlockLevel.READ, index);
222221
}
223222

224-
String[] aliases = indexNameExpressionResolver.indexAliases(
225-
clusterState,
226-
index,
227-
Predicates.always(),
228-
Predicates.always(),
229-
true,
230-
indicesAndAliases
231-
);
223+
String[] aliases = indexNameExpressionResolver.allIndexAliases(clusterState, index, indicesAndAliases);
232224
String[] finalIndices = Strings.EMPTY_ARRAY;
233225
if (aliases == null
234226
|| aliases.length == 0

‎server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import java.util.Set;
5959
import java.util.SortedMap;
6060
import java.util.function.BiFunction;
61+
import java.util.function.BiPredicate;
6162
import java.util.function.Function;
6263
import java.util.function.LongSupplier;
6364
import java.util.function.Predicate;
@@ -79,6 +80,13 @@ public class IndexNameExpressionResolver {
7980
public static final String EXCLUDED_DATA_STREAMS_KEY = "es.excluded_ds";
8081
public static final IndexVersion SYSTEM_INDEX_ENFORCEMENT_INDEX_VERSION = IndexVersions.V_8_0_0;
8182

83+
private static final BiPredicate<DataStreamAlias, Boolean> ALL_DATA_STREAM_ALIASES = (ignoredAlias, ignoredIsData) -> true;
84+
// Alias filters are not applied against indices in an abstraction's failure component.
85+
// They do not match the mapping of the data stream nor are the documents mapped for searching.
86+
private static final BiPredicate<DataStreamAlias, Boolean> ONLY_FILTERING_DATA_STREAM_ALIASES = (
87+
dataStreamAlias,
88+
isData) -> dataStreamAlias.filteringRequired() && isData;
89+
8290
private final ThreadContext threadContext;
8391
private final SystemIndices systemIndices;
8492

@@ -918,7 +926,16 @@ public Set<ResolvedExpression> resolveExpressions(
918926
* <b>NOTE</b>: The provided expressions must have been resolved already via {@link #resolveExpressionsToResources(Context, String...)}.
919927
*/
920928
public String[] filteringAliases(ClusterState state, String index, Set<ResolvedExpression> resolvedExpressions) {
921-
return indexAliases(state, index, AliasMetadata::filteringRequired, DataStreamAlias::filteringRequired, false, resolvedExpressions);
929+
return indexAliases(state, index, AliasMetadata::filteringRequired, ONLY_FILTERING_DATA_STREAM_ALIASES, false, resolvedExpressions);
930+
}
931+
932+
/**
933+
* Iterates through the list of indices and selects the effective list of all aliases for the
934+
* given index. Aliases are returned even if the index is included in the resolved expressions.
935+
* <b>NOTE</b>: The provided expressions must have been resolved already via {@link #resolveExpressions}.
936+
*/
937+
public String[] allIndexAliases(ClusterState state, String index, Set<ResolvedExpression> resolvedExpressions) {
938+
return indexAliases(state, index, Predicates.always(), ALL_DATA_STREAM_ALIASES, true, resolvedExpressions);
922939
}
923940

924941
/**
@@ -942,7 +959,7 @@ public String[] indexAliases(
942959
ClusterState state,
943960
String index,
944961
Predicate<AliasMetadata> requiredAlias,
945-
Predicate<DataStreamAlias> requiredDataStreamAlias,
962+
BiPredicate<DataStreamAlias, Boolean> requiredDataStreamAlias,
946963
boolean skipIdentity,
947964
Set<ResolvedExpression> resolvedExpressions
948965
) {
@@ -969,13 +986,8 @@ public String[] indexAliases(
969986
IndexAbstraction ia = state.metadata().getIndicesLookup().get(index);
970987
DataStream dataStream = ia.getParentDataStream();
971988
if (dataStream != null) {
972-
if (dataStream.getFailureComponent().containsIndex(index)) {
973-
// Alias filters are not applied against indices in an abstraction's failure component.
974-
// They do not match the mapping of the data stream nor are the documents mapped for searching.
975-
return null;
976-
}
977-
978-
if (skipIdentity == false && resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStream.getName())) {
989+
boolean isData = dataStream.isFailureStoreIndex(index) == false;
990+
if (skipIdentity == false && resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStream.getName(), isData)) {
979991
// skip the filters when the request targets the data stream name + selector directly
980992
return null;
981993
}
@@ -984,12 +996,14 @@ public String[] indexAliases(
984996
if (iterateIndexAliases(dataStreamAliases.size(), resolvedExpressions.size())) {
985997
aliasesForDataStream = dataStreamAliases.values()
986998
.stream()
987-
.filter(dataStreamAlias -> resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStreamAlias.getName()))
999+
.filter(
1000+
dataStreamAlias -> resolvedExpressionsContainsAbstraction(resolvedExpressions, dataStreamAlias.getName(), isData)
1001+
)
9881002
.filter(dataStreamAlias -> dataStreamAlias.getDataStreams().contains(dataStream.getName()))
9891003
.toList();
9901004
} else {
9911005
aliasesForDataStream = resolvedExpressions.stream()
992-
.filter(expression -> expression.selector() == null || expression.selector().shouldIncludeData())
1006+
.filter(expression -> (expression.selector() == null || expression.selector().shouldIncludeData()) == isData)
9931007
.map(ResolvedExpression::resource)
9941008
.map(dataStreamAliases::get)
9951009
.filter(dataStreamAlias -> dataStreamAlias != null && dataStreamAlias.getDataStreams().contains(dataStream.getName()))
@@ -998,11 +1012,14 @@ public String[] indexAliases(
9981012

9991013
List<String> requiredAliases = null;
10001014
for (DataStreamAlias dataStreamAlias : aliasesForDataStream) {
1001-
if (requiredDataStreamAlias.test(dataStreamAlias)) {
1015+
if (requiredDataStreamAlias.test(dataStreamAlias, isData)) {
10021016
if (requiredAliases == null) {
10031017
requiredAliases = new ArrayList<>(aliasesForDataStream.size());
10041018
}
1005-
requiredAliases.add(dataStreamAlias.getName());
1019+
String alias = isData
1020+
? dataStreamAlias.getName()
1021+
: combineSelector(dataStreamAlias.getName(), IndexComponentSelector.FAILURES);
1022+
requiredAliases.add(alias);
10061023
} else {
10071024
// we have a non-required alias for this data stream so no need to check further
10081025
return null;
@@ -1020,7 +1037,7 @@ public String[] indexAliases(
10201037
aliasCandidates = indexAliases.values()
10211038
.stream()
10221039
// Indices can only be referenced with a data selector, or a null selector if selectors are disabled
1023-
.filter(aliasMetadata -> resolvedExpressionsContainsAbstraction(resolvedExpressions, aliasMetadata.alias()))
1040+
.filter(aliasMetadata -> resolvedExpressionsContainsAbstraction(resolvedExpressions, aliasMetadata.alias(), true))
10241041
.toArray(AliasMetadata[]::new);
10251042
} else {
10261043
// faster to iterate resolvedExpressions
@@ -1051,9 +1068,16 @@ public String[] indexAliases(
10511068
}
10521069
}
10531070

1054-
private static boolean resolvedExpressionsContainsAbstraction(Set<ResolvedExpression> resolvedExpressions, String abstractionName) {
1055-
return resolvedExpressions.contains(new ResolvedExpression(abstractionName))
1056-
|| resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.DATA));
1071+
private static boolean resolvedExpressionsContainsAbstraction(
1072+
Set<ResolvedExpression> resolvedExpressions,
1073+
String abstractionName,
1074+
boolean isData
1075+
) {
1076+
if (isData) {
1077+
return resolvedExpressions.contains(new ResolvedExpression(abstractionName))
1078+
|| resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.DATA));
1079+
}
1080+
return resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.FAILURES));
10571081
}
10581082

10591083
/**

0 commit comments

Comments
 (0)