Skip to content

Commit 7b2afee

Browse files
gmarouligmarouli
authored
Failure store - No selectors in snapshots (elastic#120114) (elastic#120711)
In this PR we include the failure store indices while disallowing the user to use selectors in the `indices` field of the create and restore snapshot requests. **Security concerns** The create and restore snapshot require cluster privileges only, so we do not have to worry about how to handle index level security (see [Create Snapshot API](https://www.elastic.co/guide/en/elasticsearch/reference/current/create-snapshot-api.html#create-snapshot-api-prereqs) & [Restore Snapshot API](https://www.elastic.co/guide/en/elasticsearch/reference/current/restore-snapshot-api.html#restore-snapshot-api-prereqs)). **Challenges** While there are other APIs that do not support selectors but they do include the failure indices in their response such as `GET _data_stream/<target>`, `GET _data_stream/<target>/stats`, etc; the snapshot APIs are a little bit different. The reason for that is that the data stream APIs first collect only the relevant data streams and then they select the backing indices and/or the failure indices. On the other hand, the snapshot API that works with both indices and data streams cannot take such a shortcut and needs to use the `concreteIndices` method. We propose, to add a flag that when the selectors are not "allowed" can determine if we need to include the failure store or not. In the past we had something similar called the default selector, but it was more flexible and it was used a fallback always. Our goal now is to only specify the behaviour we want when the selectors are not supported. This new flag allowed also to simplify the concrete index resolution in `GET _data_stream/<target>/stats` Relates to elastic#119545
1 parent 5b8da7a commit 7b2afee

File tree

15 files changed

+178
-145
lines changed

15 files changed

+178
-145
lines changed

‎modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.elasticsearch.client.internal.Client;
3737
import org.elasticsearch.cluster.metadata.DataStream;
3838
import org.elasticsearch.cluster.metadata.DataStreamAlias;
39-
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
4039
import org.elasticsearch.common.settings.Settings;
4140
import org.elasticsearch.common.unit.ByteSizeUnit;
4241
import org.elasticsearch.index.Index;
@@ -334,13 +333,13 @@ public void testSnapshotAndRestoreInPlace() {
334333
assertThat(rolloverResponse.getNewIndex(), equalTo(DataStream.getDefaultBackingIndexName("ds", 3)));
335334
}
336335

337-
public void testFailureStoreSnapshotAndRestore() throws Exception {
336+
public void testFailureStoreSnapshotAndRestore() {
338337
String dataStreamName = "with-fs";
339338
CreateSnapshotResponse createSnapshotResponse = client.admin()
340339
.cluster()
341340
.prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, REPO, SNAPSHOT)
342341
.setWaitForCompletion(true)
343-
.setIndices(IndexNameExpressionResolver.combineSelector(dataStreamName, IndexComponentSelector.ALL_APPLICABLE))
342+
.setIndices(dataStreamName)
344343
.setIncludeGlobalState(false)
345344
.get();
346345

@@ -391,6 +390,49 @@ public void testFailureStoreSnapshotAndRestore() throws Exception {
391390
}
392391
}
393392

393+
public void testSelectorsNotAllowedInSnapshotAndRestore() {
394+
String dataStreamName = "with-fs";
395+
try {
396+
client.admin()
397+
.cluster()
398+
.prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, REPO, SNAPSHOT)
399+
.setWaitForCompletion(true)
400+
.setIndices(dataStreamName + "::" + randomFrom(IndexComponentSelector.values()).getKey())
401+
.setIncludeGlobalState(false)
402+
.get();
403+
fail("Should have failed because selectors are not allowed in snapshot creation");
404+
} catch (IllegalArgumentException e) {
405+
assertThat(
406+
e.getMessage(),
407+
containsString("Index component selectors are not supported in this context but found selector in expression")
408+
);
409+
}
410+
CreateSnapshotResponse createSnapshotResponse = client.admin()
411+
.cluster()
412+
.prepareCreateSnapshot(TEST_REQUEST_TIMEOUT, REPO, SNAPSHOT)
413+
.setWaitForCompletion(true)
414+
.setIndices("ds")
415+
.setIncludeGlobalState(false)
416+
.get();
417+
418+
RestStatus status = createSnapshotResponse.getSnapshotInfo().status();
419+
assertEquals(RestStatus.OK, status);
420+
try {
421+
client.admin()
422+
.cluster()
423+
.prepareRestoreSnapshot(TEST_REQUEST_TIMEOUT, REPO, SNAPSHOT)
424+
.setWaitForCompletion(true)
425+
.setIndices(dataStreamName + "::" + randomFrom(IndexComponentSelector.values()).getKey())
426+
.get();
427+
fail("Should have failed because selectors are not allowed in snapshot restore");
428+
} catch (IllegalArgumentException e) {
429+
assertThat(
430+
e.getMessage(),
431+
containsString("Index component selectors are not supported in this context but found selector in expression")
432+
);
433+
}
434+
}
435+
394436
public void testSnapshotAndRestoreAllIncludeSpecificDataStream() throws Exception {
395437
DocWriteResponse indexResponse = client.prepareIndex("other-ds")
396438
.setOpType(DocWriteRequest.OpType.CREATE)
@@ -1216,6 +1258,7 @@ public void testExcludeDSFromSnapshotWhenExcludingAnyOfItsIndices() {
12161258
SnapshotInfo retrievedSnapshot = getSnapshot(REPO, SNAPSHOT);
12171259
assertThat(retrievedSnapshot.dataStreams(), contains(dataStreamName));
12181260
assertThat(retrievedSnapshot.indices(), containsInAnyOrder(fsBackingIndexName));
1261+
assertThat(retrievedSnapshot.indices(), not(containsInAnyOrder(fsFailureIndexName)));
12191262

12201263
assertAcked(
12211264
safeGet(client.execute(DeleteDataStreamAction.INSTANCE, new DeleteDataStreamAction.Request(TEST_REQUEST_TIMEOUT, "*")))

‎modules/data-streams/src/main/java/org/elasticsearch/datastreams/action/DataStreamsStatsTransportAction.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.elasticsearch.action.datastreams.DataStreamsStatsAction;
1717
import org.elasticsearch.action.support.ActionFilters;
1818
import org.elasticsearch.action.support.DefaultShardOperationFailedException;
19-
import org.elasticsearch.action.support.IndexComponentSelector;
2019
import org.elasticsearch.action.support.broadcast.node.TransportBroadcastByNodeAction;
2120
import org.elasticsearch.cluster.ClusterState;
2221
import org.elasticsearch.cluster.block.ClusterBlockException;
@@ -101,17 +100,6 @@ protected ClusterBlockException checkRequestBlock(
101100
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_READ, concreteIndices);
102101
}
103102

104-
@Override
105-
protected String[] resolveConcreteIndexNames(ClusterState clusterState, DataStreamsStatsAction.Request request) {
106-
return DataStreamsActionUtil.resolveConcreteIndexNamesWithSelector(
107-
indexNameExpressionResolver,
108-
clusterState,
109-
request.indices(),
110-
IndexComponentSelector.ALL_APPLICABLE,
111-
request.indicesOptions()
112-
).toArray(String[]::new);
113-
}
114-
115103
@Override
116104
protected ShardsIterator shards(ClusterState clusterState, DataStreamsStatsAction.Request request, String[] concreteIndices) {
117105
return clusterState.getRoutingTable().allShards(concreteIndices);

‎server/src/main/java/org/elasticsearch/TransportVersions.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ static TransportVersion def(int id) {
167167
public static final TransportVersion ESQL_SKIP_ES_INDEX_SERIALIZATION = def(8_827_00_0);
168168
public static final TransportVersion ADD_INDEX_BLOCK_TWO_PHASE = def(8_828_00_0);
169169
public static final TransportVersion RESOLVE_CLUSTER_NO_INDEX_EXPRESSION = def(8_829_00_0);
170+
public static final TransportVersion ML_ROLLOVER_LEGACY_INDICES = def(8_830_00_0);
171+
public static final TransportVersion ADD_INCLUDE_FAILURE_INDICES_OPTION = def(8_831_00_0);
170172

171173
/*
172174
* STOP! READ THIS FIRST! No, really,

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.elasticsearch.action.IndicesRequest;
1717
import org.elasticsearch.action.support.IndicesOptions;
1818
import org.elasticsearch.action.support.master.MasterNodeRequest;
19-
import org.elasticsearch.cluster.metadata.DataStream;
2019
import org.elasticsearch.common.Strings;
2120
import org.elasticsearch.common.UUIDs;
2221
import org.elasticsearch.common.bytes.BytesReference;
@@ -69,9 +68,7 @@ public class CreateSnapshotRequest extends MasterNodeRequest<CreateSnapshotReque
6968

7069
private String[] indices = EMPTY_ARRAY;
7170

72-
private IndicesOptions indicesOptions = DataStream.isFailureStoreFeatureFlagEnabled()
73-
? IndicesOptions.strictExpandHiddenIncludeFailureStore()
74-
: IndicesOptions.strictExpandHidden();
71+
private IndicesOptions indicesOptions = IndicesOptions.strictExpandHiddenFailureNoSelectors();
7572

7673
private String[] featureStates = EMPTY_ARRAY;
7774

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.action.ActionRequestValidationException;
1515
import org.elasticsearch.action.support.IndicesOptions;
1616
import org.elasticsearch.action.support.master.MasterNodeRequest;
17+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1718
import org.elasticsearch.common.Strings;
1819
import org.elasticsearch.common.io.stream.StreamInput;
1920
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -43,7 +44,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
4344
private String snapshot;
4445
private String repository;
4546
private String[] indices = Strings.EMPTY_ARRAY;
46-
private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen();
47+
private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpenFailureNoSelectors();
4748
private String[] featureStates = Strings.EMPTY_ARRAY;
4849
private String renamePattern;
4950
private String renameReplacement;
@@ -138,6 +139,16 @@ public ActionRequestValidationException validate() {
138139
if (indicesOptions == null) {
139140
validationException = addValidationError("indicesOptions is missing", validationException);
140141
}
142+
// This action does not use the IndexNameExpressionResolver to resolve concrete indices, this is why we check here for selectors
143+
if (indicesOptions.allowSelectors() == false) {
144+
for (String index : indices) {
145+
try {
146+
IndexNameExpressionResolver.SelectorResolver.parseExpression(index, indicesOptions);
147+
} catch (IllegalArgumentException e) {
148+
validationException = addValidationError(e.getMessage(), validationException);
149+
}
150+
}
151+
}
141152
if (featureStates == null) {
142153
validationException = addValidationError("featureStates is missing", validationException);
143154
}

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

Lines changed: 9 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
package org.elasticsearch.action.datastreams;
1111

12-
import org.elasticsearch.action.support.IndexComponentSelector;
1312
import org.elasticsearch.action.support.IndicesOptions;
1413
import org.elasticsearch.cluster.ClusterState;
1514
import org.elasticsearch.cluster.metadata.DataStream;
@@ -55,64 +54,24 @@ public static List<String> resolveConcreteIndexNames(
5554
String[] names,
5655
IndicesOptions indicesOptions
5756
) {
58-
List<ResolvedExpression> abstractionNames = indexNameExpressionResolver.dataStreams(
57+
List<ResolvedExpression> resolvedDataStreamExpressions = indexNameExpressionResolver.dataStreams(
5958
clusterState,
6059
updateIndicesOptions(indicesOptions),
6160
names
6261
);
6362
SortedMap<String, IndexAbstraction> indicesLookup = clusterState.getMetadata().getIndicesLookup();
6463

65-
List<String> results = new ArrayList<>(abstractionNames.size());
66-
for (ResolvedExpression abstractionName : abstractionNames) {
67-
IndexAbstraction indexAbstraction = indicesLookup.get(abstractionName.resource());
64+
List<String> results = new ArrayList<>(resolvedDataStreamExpressions.size());
65+
for (ResolvedExpression resolvedExpression : resolvedDataStreamExpressions) {
66+
IndexAbstraction indexAbstraction = indicesLookup.get(resolvedExpression.resource());
6867
assert indexAbstraction != null;
6968
if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) {
70-
selectDataStreamIndicesNames(
71-
(DataStream) indexAbstraction,
72-
IndexComponentSelector.FAILURES.equals(abstractionName.selector()),
73-
results
74-
);
75-
}
76-
}
77-
return results;
78-
}
79-
80-
/**
81-
* Resolves a list of expressions into data stream names and then collects the concrete indices
82-
* that are applicable for those data streams based on the selector provided in the arguments.
83-
* @param indexNameExpressionResolver resolver object
84-
* @param clusterState state to query
85-
* @param names data stream expressions
86-
* @param selector which component indices of the data stream should be returned
87-
* @param indicesOptions options for expression resolution
88-
* @return A stream of concrete index names that belong to the components specified
89-
* on the data streams returned from the expressions given
90-
*/
91-
public static List<String> resolveConcreteIndexNamesWithSelector(
92-
IndexNameExpressionResolver indexNameExpressionResolver,
93-
ClusterState clusterState,
94-
String[] names,
95-
IndexComponentSelector selector,
96-
IndicesOptions indicesOptions
97-
) {
98-
assert indicesOptions.allowSelectors() == false : "If selectors are enabled, use resolveConcreteIndexNames instead";
99-
List<String> abstractionNames = indexNameExpressionResolver.dataStreamNames(
100-
clusterState,
101-
updateIndicesOptions(indicesOptions),
102-
names
103-
);
104-
SortedMap<String, IndexAbstraction> indicesLookup = clusterState.getMetadata().getIndicesLookup();
105-
106-
List<String> results = new ArrayList<>(abstractionNames.size());
107-
for (String abstractionName : abstractionNames) {
108-
IndexAbstraction indexAbstraction = indicesLookup.get(abstractionName);
109-
assert indexAbstraction != null;
110-
if (indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) {
111-
if (selector.shouldIncludeData()) {
112-
selectDataStreamIndicesNames((DataStream) indexAbstraction, false, results);
69+
DataStream dataStream = (DataStream) indexAbstraction;
70+
if (IndexNameExpressionResolver.shouldIncludeRegularIndices(indicesOptions, resolvedExpression.selector())) {
71+
selectDataStreamIndicesNames(dataStream, false, results);
11372
}
114-
if (selector.shouldIncludeFailures()) {
115-
selectDataStreamIndicesNames((DataStream) indexAbstraction, true, results);
73+
if (IndexNameExpressionResolver.shouldIncludeFailureIndices(indicesOptions, resolvedExpression.selector())) {
74+
selectDataStreamIndicesNames(dataStream, true, results);
11675
}
11776
}
11877
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public Request() {
5757
.allowClosedIndices(true)
5858
.ignoreThrottled(false)
5959
.allowSelectors(false)
60+
.includeFailureIndices(true)
6061
.build()
6162
)
6263
.build()

0 commit comments

Comments
 (0)