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 @@ -186,6 +186,7 @@ static TransportVersion def(int id) {
public static final TransportVersion ESQL_RETRY_ON_SHARD_LEVEL_FAILURE = def(9_006_0_00);
public static final TransportVersion ESQL_PROFILE_ASYNC_NANOS = def(9_007_00_0);
public static final TransportVersion ESQL_LOOKUP_JOIN_SOURCE_TEXT = def(9_008_0_00);
public static final TransportVersion REMOVE_ALL_APPLICABLE_SELECTOR = def(9_009_0_00);

/*
* STOP! READ THIS FIRST! No, really,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -646,10 +646,6 @@ private static void enrichIndexAbstraction(
: switch (resolvedExpression.selector()) {
case DATA -> dataStream.getDataComponent().getIndices().stream();
case FAILURES -> dataStream.getFailureIndices().stream();
case ALL_APPLICABLE -> Stream.concat(
dataStream.getIndices().stream(),
dataStream.getFailureIndices().stream()
);
};
String[] backingIndices = dataStreamIndices.map(Index::getName).toArray(String[]::new);
dataStreams.add(new ResolvedDataStream(dataStream.getName(), backingIndices, DataStream.TIMESTAMP_FIELD_NAME));
Expand All @@ -670,13 +666,6 @@ private static Stream<Index> getAliasIndexStream(ResolvedExpression resolvedExpr
assert ia.isDataStreamRelated() : "Illegal selector [failures] used on non data stream alias";
yield ia.getFailureIndices(metadata).stream();
}
case ALL_APPLICABLE -> {
if (ia.isDataStreamRelated()) {
yield Stream.concat(ia.getIndices().stream(), ia.getFailureIndices(metadata).stream());
} else {
yield ia.getIndices().stream();
}
}
};
}
return aliasIndices;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndexComponentSelector;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.SelectorResolver;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.indices.InvalidIndexNameException;
import org.elasticsearch.tasks.CancellableTask;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.tasks.TaskId;
Expand Down Expand Up @@ -126,14 +125,12 @@ public ActionRequestValidationException validate() {
);
}

// Ensure we have a valid selector in the request
if (rolloverTarget != null) {
ResolvedExpression resolvedExpression = SelectorResolver.parseExpression(rolloverTarget, indicesOptions);
IndexComponentSelector selector = resolvedExpression.selector();
if (IndexComponentSelector.ALL_APPLICABLE.equals(selector)) {
validationException = addValidationError(
"rollover cannot be applied to both regular and failure indices at the same time",
validationException
);
try {
SelectorResolver.parseExpression(rolloverTarget, indicesOptions);
} catch (InvalidIndexNameException exception) {
validationException = addValidationError(exception.getMessage(), validationException);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

package org.elasticsearch.action.support;

import org.elasticsearch.TransportVersions;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
Expand All @@ -23,14 +24,11 @@
* We define as index components the two different sets of indices a data stream could consist of:
* - DATA: represents the backing indices
* - FAILURES: represent the failing indices
* - ALL: represents all available in this expression components, meaning if it's a data stream both backing and failure indices and if it's
* an index only the index itself.
* Note: An index is its own DATA component, but it cannot have a FAILURE component.
*/
public enum IndexComponentSelector implements Writeable {
DATA("data", (byte) 0),
FAILURES("failures", (byte) 1),
ALL_APPLICABLE("*", (byte) 2);
FAILURES("failures", (byte) 1);
Copy link
Member

Choose a reason for hiding this comment

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

This is an unlikely scenario but could we ever end up in a place where a new selector added here could collide with the old ::* value in a mixed cluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm.... Interesting thought. Thinking out loud because this is a tough cookie.

If we implement this the way it's now and backport it back to 8.18 and 8.x.
We can remove this backwards compatibility code from 9.x; because you always have to go through the 8.last version. 8.last will never propagate this selector because it matches it to ::data and this selector is never persisted anywhere, I think. Furthermore, there is no CCR or CCS supported yet. Right?

This means that if we ever introduced ::* in 9+ we could distinguish the two. Right?


private final String key;
private final byte id;
Expand Down Expand Up @@ -75,7 +73,13 @@ public static IndexComponentSelector getByKey(String key) {
}

public static IndexComponentSelector read(StreamInput in) throws IOException {
return getById(in.readByte());
byte id = in.readByte();
if (in.getTransportVersion().onOrAfter(TransportVersions.REMOVE_ALL_APPLICABLE_SELECTOR)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

More versions will be added here when it gets backported.

return getById(id);
} else {
// Legacy value ::*, converted to ::data
return id == 2 ? DATA : getById(id);
}
}

// Visible for testing
Expand All @@ -95,10 +99,10 @@ public void writeTo(StreamOutput out) throws IOException {
}

public boolean shouldIncludeData() {
return this == ALL_APPLICABLE || this == DATA;
return this == DATA;
}

public boolean shouldIncludeFailures() {
return this == ALL_APPLICABLE || this == FAILURES;
return this == FAILURES;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ && isIndexVisible(
indexNameExpressionResolver,
includeDataStreams
)) {
// Resolve any ::* suffixes on the expression. We need to resolve them all to their final valid selectors
resolveSelectorsAndCombine(authorizedIndex, selectorString, indicesOptions, resolvedIndices, metadata);
resolveSelectorsAndCollect(authorizedIndex, selectorString, indicesOptions, resolvedIndices, metadata);
}
}
if (resolvedIndices.isEmpty()) {
Expand All @@ -98,9 +97,8 @@ && isIndexVisible(
}
}
} else {
// Resolve any ::* suffixes on the expression. We need to resolve them all to their final valid selectors
Set<String> resolvedIndices = new HashSet<>();
resolveSelectorsAndCombine(indexAbstraction, selectorString, indicesOptions, resolvedIndices, metadata);
resolveSelectorsAndCollect(indexAbstraction, selectorString, indicesOptions, resolvedIndices, metadata);
if (minus) {
finalIndices.removeAll(resolvedIndices);
} else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction)) {
Expand All @@ -114,7 +112,7 @@ && isIndexVisible(
return finalIndices;
}

private static void resolveSelectorsAndCombine(
private static void resolveSelectorsAndCollect(
String indexAbstraction,
String selectorString,
IndicesOptions indicesOptions,
Expand All @@ -132,19 +130,8 @@ private static void resolveSelectorsAndCombine(
selectorString = IndexComponentSelector.DATA.getKey();
}

if (Regex.isMatchAllPattern(selectorString)) {
// Always accept data
collect.add(IndexNameExpressionResolver.combineSelectorExpression(indexAbstraction, IndexComponentSelector.DATA.getKey()));
// Only put failures on the expression if the abstraction supports it.
if (acceptsAllSelectors) {
collect.add(
IndexNameExpressionResolver.combineSelectorExpression(indexAbstraction, IndexComponentSelector.FAILURES.getKey())
);
}
} else {
// A non-wildcard selector is always passed along as-is, it's validity for this kind of abstraction is tested later
collect.add(IndexNameExpressionResolver.combineSelectorExpression(indexAbstraction, selectorString));
}
// A selector is always passed along as-is, it's validity for this kind of abstraction is tested later
collect.add(IndexNameExpressionResolver.combineSelectorExpression(indexAbstraction, selectorString));
} else {
assert selectorString == null
: "A selector string [" + selectorString + "] is present but selectors are disabled in this context";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,21 +364,9 @@ protected static Collection<ResolvedExpression> resolveExpressionsToResources(Co
}
} else {
if (isExclusion) {
if (IndexComponentSelector.ALL_APPLICABLE.equals(selector)) {
resources.remove(new ResolvedExpression(baseExpression, IndexComponentSelector.DATA));
resources.remove(new ResolvedExpression(baseExpression, IndexComponentSelector.FAILURES));
} else {
resources.remove(new ResolvedExpression(baseExpression, selector));
}
resources.remove(new ResolvedExpression(baseExpression, selector));
} else if (ensureAliasOrIndexExists(context, baseExpression, selector)) {
if (IndexComponentSelector.ALL_APPLICABLE.equals(selector)) {
resources.add(new ResolvedExpression(baseExpression, IndexComponentSelector.DATA));
if (context.getState().getMetadata().getIndicesLookup().get(baseExpression).isDataStreamRelated()) {
resources.add(new ResolvedExpression(baseExpression, IndexComponentSelector.FAILURES));
}
} else {
resources.add(new ResolvedExpression(baseExpression, selector));
}
resources.add(new ResolvedExpression(baseExpression, selector));
}
}
}
Expand Down Expand Up @@ -1046,8 +1034,7 @@ 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))
|| resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.ALL_APPLICABLE));
|| resolvedExpressions.contains(new ResolvedExpression(abstractionName, IndexComponentSelector.DATA));
}

/**
Expand Down Expand Up @@ -1342,8 +1329,7 @@ private static boolean ensureAliasOrIndexExists(Context context, String name, In
if (context.options.allowSelectors()) {
// Ensure that the selectors are present and that they are compatible with the abstractions they are used with
assert selector != null : "Earlier logic should have parsed selectors or added the default selectors already";
// Check if ::failures has been explicitly requested, since requesting ::* for non-data-stream abstractions would just
// return their data components.
// Check if ::failures has been explicitly requested
if (IndexComponentSelector.FAILURES.equals(selector) && indexAbstraction.isDataStreamRelated() == false) {
// If requested abstraction is not data stream related, then you cannot use ::failures
if (ignoreUnavailable) {
Expand Down Expand Up @@ -1700,9 +1686,9 @@ private static Set<ResolvedExpression> expandToOpenClosed(
final IndexMetadata.State excludeState = excludeState(context.getOptions());
Set<ResolvedExpression> resources = new HashSet<>();
if (context.isPreserveAliases() && indexAbstraction.getType() == Type.ALIAS) {
expandToApplicableSelectors(indexAbstraction, selector, resources);
resources.add(new ResolvedExpression(indexAbstraction.getName(), selector));
} else if (context.isPreserveDataStreams() && indexAbstraction.getType() == Type.DATA_STREAM) {
expandToApplicableSelectors(indexAbstraction, selector, resources);
resources.add(new ResolvedExpression(indexAbstraction.getName(), selector));
} else {
if (shouldIncludeRegularIndices(context.getOptions(), selector)) {
for (int i = 0, n = indexAbstraction.getIndices().size(); i < n; i++) {
Expand All @@ -1729,31 +1715,6 @@ private static Set<ResolvedExpression> expandToOpenClosed(
return resources;
}

/**
* Adds the abstraction and selector to the results when preserving data streams and aliases at wildcard resolution. If a selector
* is provided, the result is only added if the selector is applicable to the abstraction provided. If
* {@link IndexComponentSelector#ALL_APPLICABLE} is given, the selectors are expanded only to those which are applicable to the
* provided abstraction.
* @param indexAbstraction abstraction to add
* @param selector The selector to add
* @param resources Result collector which is updated with all applicable resolved expressions for a given abstraction and selector
* pair.
*/
private static void expandToApplicableSelectors(
IndexAbstraction indexAbstraction,
IndexComponentSelector selector,
Set<ResolvedExpression> resources
) {
if (IndexComponentSelector.ALL_APPLICABLE.equals(selector)) {
resources.add(new ResolvedExpression(indexAbstraction.getName(), IndexComponentSelector.DATA));
if (indexAbstraction.isDataStreamRelated()) {
resources.add(new ResolvedExpression(indexAbstraction.getName(), IndexComponentSelector.FAILURES));
}
} else if (selector == null || indexAbstraction.isDataStreamRelated() || selector.shouldIncludeFailures() == false) {
resources.add(new ResolvedExpression(indexAbstraction.getName(), selector));
}
}

private static List<ResolvedExpression> resolveEmptyOrTrivialWildcard(Context context, IndexComponentSelector selector) {
final String[] allIndices = resolveEmptyOrTrivialWildcardToAllIndices(
context.getOptions(),
Expand Down Expand Up @@ -2150,20 +2111,10 @@ private static <V> V splitSelectorExpression(String expression, BiFunction<Strin
String suffix = expression.substring(lastDoubleColon + SELECTOR_SEPARATOR.length());
IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix);
if (selector == null) {
// Do some work to surface a helpful error message for likely errors
if (Regex.isSimpleMatchPattern(suffix)) {
throw new InvalidIndexNameException(
expression,
"Invalid usage of :: separator, ["
+ suffix
+ "] contains a wildcard, but only the match all wildcard [*] is supported in a selector"
);
} else {
throw new InvalidIndexNameException(
expression,
"Invalid usage of :: separator, [" + suffix + "] is not a recognized selector"
);
}
throw new InvalidIndexNameException(
expression,
"invalid usage of :: separator, [" + suffix + "] is not a recognized selector"
);
}
String expressionBase = expression.substring(0, lastDoubleColon);
ensureNoMoreSelectorSeparators(expressionBase, expression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,15 @@ protected Map<String, List<String>> groupClusterIndices(Set<String> remoteCluste
if (indexName.equals("*") == false) {
throw new IllegalArgumentException(
Strings.format(
"To exclude a cluster you must specify the '*' wildcard for " + "the index expression, but found: [%s]",
"To exclude a cluster you must specify the '*' wildcard for the index expression, but found: [%s]",
indexName
)
);
}
if (selectorString != null && selectorString.equals("*") == false) {
if (selectorString != null) {
throw new IllegalArgumentException(
Strings.format(
"To exclude a cluster you must specify the '::*' selector or leave it off, but found: [%s]",
"To exclude a cluster you must not specify the a selector, but found selector: [%s]",
selectorString
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ public void testValidation() {
assertNotNull(validationException);
assertEquals(1, validationException.validationErrors().size());
assertEquals(
"rollover cannot be applied to both regular and failure indices at the same time",
"Invalid index name [alias-index::*], invalid usage of :: separator, [*] is not a recognized selector",
validationException.validationErrors().get(0)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class IndexComponentSelectorTests extends ESTestCase {
public void testIndexComponentSelectorFromKey() {
assertThat(IndexComponentSelector.getByKey("data"), equalTo(IndexComponentSelector.DATA));
assertThat(IndexComponentSelector.getByKey("failures"), equalTo(IndexComponentSelector.FAILURES));
assertThat(IndexComponentSelector.getByKey("*"), equalTo(IndexComponentSelector.ALL_APPLICABLE));
assertThat(IndexComponentSelector.getByKey("*"), nullValue());
assertThat(IndexComponentSelector.getByKey("d*ta"), nullValue());
assertThat(IndexComponentSelector.getByKey("_all"), nullValue());
assertThat(IndexComponentSelector.getByKey("**"), nullValue());
Expand All @@ -30,11 +30,10 @@ public void testIndexComponentSelectorFromKey() {
public void testIndexComponentSelectorFromId() {
assertThat(IndexComponentSelector.getById((byte) 0), equalTo(IndexComponentSelector.DATA));
assertThat(IndexComponentSelector.getById((byte) 1), equalTo(IndexComponentSelector.FAILURES));
assertThat(IndexComponentSelector.getById((byte) 2), equalTo(IndexComponentSelector.ALL_APPLICABLE));
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> IndexComponentSelector.getById((byte) 3));
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> IndexComponentSelector.getById((byte) 2));
assertThat(
exception.getMessage(),
containsString("Unknown id of index component selector [3], available options are: {0=DATA, 1=FAILURES, 2=ALL_APPLICABLE}")
containsString("Unknown id of index component selector [2], available options are: {0=DATA, 1=FAILURES}")
);
}

Expand Down
Loading