Skip to content

Commit 9f529c6

Browse files
jbaierajbaiera
committed
Fix another round of bugs and tests
1 parent 92c818e commit 9f529c6

File tree

5 files changed

+35
-78
lines changed

5 files changed

+35
-78
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
4444
private String snapshot;
4545
private String repository;
4646
private String[] indices = Strings.EMPTY_ARRAY;
47-
private IndicesOptions indicesOptions = DataStream.isFailureStoreFeatureFlagEnabled()
48-
? IndicesOptions.strictExpandOpenIncludeFailureStore()
49-
: IndicesOptions.strictExpandOpen();
47+
private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen();
5048
private String[] featureStates = Strings.EMPTY_ARRAY;
5149
private String renamePattern;
5250
private String renameReplacement;

‎server/src/main/java/org/elasticsearch/action/admin/indices/rollover/LazyRolloverAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ protected void masterOperation(
130130

131131
DataStream dataStream = metadata.dataStreams().get(resolvedRolloverTarget.resource());
132132
// Skip submitting the task if we detect that the lazy rollover has been already executed.
133-
if (isLazyRolloverNeeded(dataStream, isFailureStoreRollover)) {
133+
if (isLazyRolloverNeeded(dataStream, isFailureStoreRollover) == false) {
134134
DataStream.DataStreamIndices targetIndices = dataStream.getDataStreamIndices(isFailureStoreRollover);
135135
listener.onResponse(noopLazyRolloverResponse(targetIndices));
136136
return;

‎server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -883,8 +883,10 @@ public void writeIndicesOptions(StreamOutput out) throws IOException {
883883
if (ignoreUnavailable()) {
884884
backwardsCompatibleOptions.add(Option.ALLOW_UNAVAILABLE_CONCRETE_TARGETS);
885885
}
886-
if (out.getTransportVersion().onOrAfter(TransportVersions.REPLACE_FAILURE_STORE_OPTIONS_WITH_SELECTOR_SYNTAX)) {
887-
if (allowSelectors()) {
886+
if (allowSelectors()) {
887+
if (out.getTransportVersion().between(TransportVersions.V_8_14_0, TransportVersions.REPLACE_FAILURE_STORE_OPTIONS_WITH_SELECTOR_SYNTAX)) {
888+
backwardsCompatibleOptions.add(Option.ALLOW_FAILURE_INDICES);
889+
} else if (out.getTransportVersion().onOrAfter(TransportVersions.REPLACE_FAILURE_STORE_OPTIONS_WITH_SELECTOR_SYNTAX)) {
888890
backwardsCompatibleOptions.add(Option.ALLOW_SELECTORS);
889891
}
890892
}
@@ -924,8 +926,12 @@ public static IndicesOptions readIndicesOptions(StreamInput in) throws IOExcepti
924926
options.contains(Option.EXCLUDE_ALIASES)
925927
);
926928
boolean allowSelectors = true;
927-
// PRTODO: Lets be smarter about allowSelectors default here.
928-
if (in.getTransportVersion().onOrAfter(TransportVersions.REPLACE_FAILURE_STORE_OPTIONS_WITH_SELECTOR_SYNTAX)) {
929+
if (in.getTransportVersion()
930+
.between(TransportVersions.V_8_14_0, TransportVersions.REPLACE_FAILURE_STORE_OPTIONS_WITH_SELECTOR_SYNTAX)) {
931+
// We've effectively replaced the allow failure indices setting with allow selectors. If it is configured on an older version
932+
// then use its value for allow selectors.
933+
allowSelectors = options.contains(Option.ALLOW_FAILURE_INDICES);
934+
} else if (in.getTransportVersion().onOrAfter(TransportVersions.REPLACE_FAILURE_STORE_OPTIONS_WITH_SELECTOR_SYNTAX)) {
929935
allowSelectors = options.contains(Option.ALLOW_SELECTORS);
930936
}
931937
GatekeeperOptions gatekeeperOptions = GatekeeperOptions.builder()
@@ -1311,14 +1317,6 @@ public static IndicesOptions strictExpandOpen() {
13111317
return STRICT_EXPAND_OPEN;
13121318
}
13131319

1314-
/**
1315-
* @return indices options that requires every specified index to exist, expands wildcards only to open indices and
1316-
* allows that no indices are resolved from wildcard expressions (not returning an error).
1317-
*/
1318-
public static IndicesOptions strictExpandOpenIncludeFailureStore() {
1319-
return STRICT_EXPAND_OPEN_FAILURE_STORE;
1320-
}
1321-
13221320
/**
13231321
* @return indices options that requires every specified index to exist, expands wildcards only to open indices,
13241322
* allows that no indices are resolved from wildcard expressions (not returning an error) and forbids the

‎server/src/test/java/org/elasticsearch/action/OriginalIndicesTests.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,9 @@ public void testOriginalIndicesSerialization() throws IOException {
4444

4545
assertThat(originalIndices2.indices(), equalTo(originalIndices.indices()));
4646
// indices options are not equivalent when sent to an older version and re-read due
47-
// to the addition of hidden indices as expand to hidden indices is always true when
48-
// read from a prior version
49-
if (out.getTransportVersion().onOrAfter(TransportVersions.V_7_7_0)
50-
|| originalIndices.indicesOptions().expandWildcardsHidden()) {
51-
assertThat(originalIndices2.indicesOptions(), equalTo(originalIndices.indicesOptions()));
52-
} else if (originalIndices.indicesOptions().expandWildcardsHidden()) {
47+
// to the addition of selector settings. Allow selectors is always true when read
48+
// from a version prior to its addition, since true is the default value.
49+
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_14_0) || originalIndices.indicesOptions().allowSelectors()) {
5350
assertThat(originalIndices2.indicesOptions(), equalTo(originalIndices.indicesOptions()));
5451
}
5552
}

‎server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java‎

Lines changed: 20 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,11 @@ public void testResolveIndexAbstractions() {
7070
assertThat(resolveAbstractionsSelectorNotAllowed(List.of("index1")), contains("index1"));
7171
// No selectors allowed, valid selector given
7272
expectThrows(IllegalArgumentException.class, () -> resolveAbstractionsSelectorNotAllowed(List.of("index1::data")));
73-
// Selectors allowed, valid selector given
74-
assertThat(resolveAbstractionsSelectorAllowed(List.of("index1::data")), contains("index1::data"));
75-
// Selectors allowed, wildcard selector provided
76-
// ** only returns ::data since expression is an index
77-
assertThat(resolveAbstractionsSelectorAllowed(List.of("index1::*")), contains("index1::data"));
78-
// Selectors allowed, none given, default to both selectors
73+
// Selectors allowed, valid selector given, data selector stripped off in result since it is the default
74+
assertThat(resolveAbstractionsSelectorAllowed(List.of("index1::data")), contains("index1"));
75+
// Selectors allowed, wildcard selector provided, data selector stripped off in result since it is the default
7976
// ** only returns ::data since expression is an index
80-
assertThat(resolveAbstractionsDefaultBothSelector(List.of("index1::*")), contains("index1::data"));
77+
assertThat(resolveAbstractionsSelectorAllowed(List.of("index1::*")), contains("index1"));
8178
// Selectors allowed, invalid selector given
8279
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("index1::custom")));
8380

@@ -93,24 +90,18 @@ public void testResolveIndexAbstractions() {
9390
// Selectors allowed, none given
9491
assertThat(
9592
resolveAbstractionsSelectorAllowed(List.of("<datetime-{now/M}>")),
96-
contains(either(equalTo(dateTimeIndexToday + "::data")).or(equalTo(dateTimeIndexTomorrow + "::data")))
93+
contains(either(equalTo(dateTimeIndexToday)).or(equalTo(dateTimeIndexTomorrow)))
9794
);
98-
// Selectors allowed, valid selector provided
95+
// Selectors allowed, valid selector provided, data selector stripped off in result since it is the default
9996
assertThat(
10097
resolveAbstractionsSelectorAllowed(List.of("<datetime-{now/M}>::data")),
101-
contains(either(equalTo(dateTimeIndexToday + "::data")).or(equalTo(dateTimeIndexTomorrow + "::data")))
98+
contains(either(equalTo(dateTimeIndexToday)).or(equalTo(dateTimeIndexTomorrow)))
10299
);
103-
// Selectors allowed, wildcard selector provided
100+
// Selectors allowed, wildcard selector provided, data selector stripped off in result since it is the default
104101
// ** only returns ::data since expression is an index
105102
assertThat(
106103
resolveAbstractionsSelectorAllowed(List.of("<datetime-{now/M}>::data")),
107-
contains(either(equalTo(dateTimeIndexToday + "::data")).or(equalTo(dateTimeIndexTomorrow + "::data")))
108-
);
109-
// Selectors allowed, none given, default to both selectors
110-
// ** only returns ::data since expression is an index
111-
assertThat(
112-
resolveAbstractionsDefaultBothSelector(List.of("<datetime-{now/M}>::data")),
113-
contains(either(equalTo(dateTimeIndexToday + "::data")).or(equalTo(dateTimeIndexTomorrow + "::data")))
104+
contains(either(equalTo(dateTimeIndexToday)).or(equalTo(dateTimeIndexTomorrow)))
114105
);
115106
// Selectors allowed, invalid selector given
116107
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("<datetime-{now/M}>::custom")));
@@ -121,14 +112,11 @@ public void testResolveIndexAbstractions() {
121112
assertThat(resolveAbstractionsSelectorNotAllowed(List.of("index*")), containsInAnyOrder("index1", "index2"));
122113
// No selectors allowed, valid selector given
123114
expectThrows(IllegalArgumentException.class, () -> resolveAbstractionsSelectorNotAllowed(List.of("index*::data")));
124-
// Selectors allowed, valid selector given
125-
assertThat(resolveAbstractionsSelectorAllowed(List.of("index*::data")), containsInAnyOrder("index1::data", "index2::data"));
126-
// Selectors allowed, wildcard selector provided
127-
// ** only returns ::data since expression is an index
128-
assertThat(resolveAbstractionsSelectorAllowed(List.of("index*::*")), containsInAnyOrder("index1::data", "index2::data"));
129-
// Selectors allowed, none given, default to both selectors
115+
// Selectors allowed, valid selector given, data selector stripped off in result since it is the default
116+
assertThat(resolveAbstractionsSelectorAllowed(List.of("index*::data")), containsInAnyOrder("index1", "index2"));
117+
// Selectors allowed, wildcard selector provided, data selector stripped off in result since it is the default
130118
// ** only returns ::data since expression is an index
131-
assertThat(resolveAbstractionsDefaultBothSelector(List.of("index*::*")), containsInAnyOrder("index1::data", "index2::data"));
119+
assertThat(resolveAbstractionsSelectorAllowed(List.of("index*::*")), containsInAnyOrder("index1", "index2"));
132120
// Selectors allowed, invalid selector given
133121
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("index*::custom")));
134122

@@ -142,15 +130,10 @@ public void testResolveIndexAbstractions() {
142130
assertThat(resolveAbstractionsSelectorAllowed(List.of("data-stream1::failures")), contains("data-stream1::failures"));
143131
// Selectors allowed, wildcard selector provided
144132
// ** returns both ::data and ::failures since expression is a data stream
133+
// ** data selector stripped off in result since it is the default
145134
assertThat(
146135
resolveAbstractionsSelectorAllowed(List.of("data-stream1::*")),
147-
containsInAnyOrder("data-stream1::data", "data-stream1::failures")
148-
);
149-
// Selectors allowed, none given, default to both selectors
150-
// ** returns both ::data and ::failures since expression is a data stream
151-
assertThat(
152-
resolveAbstractionsDefaultBothSelector(List.of("data-stream1::*")),
153-
containsInAnyOrder("data-stream1::data", "data-stream1::failures")
136+
containsInAnyOrder("data-stream1", "data-stream1::failures")
154137
);
155138
// Selectors allowed, invalid selector given
156139
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("data-stream1::custom")));
@@ -167,13 +150,7 @@ public void testResolveIndexAbstractions() {
167150
// ** returns both ::data and ::failures since expression is a data stream
168151
assertThat(
169152
resolveAbstractionsSelectorAllowed(List.of("data-stream*::*")),
170-
containsInAnyOrder("data-stream1::data", "data-stream1::failures")
171-
);
172-
// Selectors allowed, none given, default to both selectors
173-
// ** returns both ::data and ::failures since expression is a data stream
174-
assertThat(
175-
resolveAbstractionsDefaultBothSelector(List.of("data-stream*::*")),
176-
containsInAnyOrder("data-stream1::data", "data-stream1::failures")
153+
containsInAnyOrder("data-stream1", "data-stream1::failures")
177154
);
178155
// Selectors allowed, invalid selector given
179156
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("data-stream*::custom")));
@@ -188,7 +165,7 @@ public void testResolveIndexAbstractions() {
188165
// ::data selector returns all values with data component
189166
assertThat(
190167
resolveAbstractionsSelectorAllowed(List.of("*::data")),
191-
containsInAnyOrder("index1::data", "index2::data", "data-stream1::data")
168+
containsInAnyOrder("index1", "index2", "data-stream1")
192169
);
193170
// Selectors allowed, valid selector given
194171
// ::failures selector returns only data streams, which can have failure components
@@ -197,13 +174,7 @@ public void testResolveIndexAbstractions() {
197174
// ** returns both ::data and ::failures for applicable abstractions
198175
assertThat(
199176
resolveAbstractionsSelectorAllowed(List.of("*::*")),
200-
containsInAnyOrder("index1::data", "index2::data", "data-stream1::data", "data-stream1::failures")
201-
);
202-
// Selectors allowed, none given, default to both selectors
203-
// ** returns both ::data and ::failures for applicable abstractions
204-
assertThat(
205-
resolveAbstractionsDefaultBothSelector(List.of("*")),
206-
containsInAnyOrder("index1::data", "index2::data", "data-stream1::data", "data-stream1::failures")
177+
containsInAnyOrder("index1", "index2", "data-stream1", "data-stream1::failures")
207178
);
208179
// Selectors allowed, invalid selector given
209180
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("*::custom")));
@@ -222,16 +193,13 @@ public void testResolveIndexAbstractions() {
222193
// ** limits the returned values based on selectors
223194
assertThat(
224195
resolveAbstractionsSelectorAllowed(List.of("*::*", "-*::failures")),
225-
containsInAnyOrder("index1::data", "index2::data", "data-stream1::data")
196+
containsInAnyOrder("index1", "index2", "data-stream1")
226197
);
227198
// Selectors allowed, none given, default to both selectors
228199
// ** limits the returned values based on selectors
229-
assertThat(resolveAbstractionsDefaultBothSelector(List.of("*", "-*::data")), containsInAnyOrder("data-stream1::failures"));
230-
// Selectors allowed, none given, default to both selectors
231-
// ** limits the returned values based on selectors
232200
assertThat(
233201
resolveAbstractionsSelectorAllowed(List.of("*", "-*::failures")),
234-
containsInAnyOrder("index1::data", "index2::data", "data-stream1::data")
202+
containsInAnyOrder("index1", "index2", "data-stream1")
235203
);
236204
// Selectors allowed, invalid selector given
237205
expectThrows(InvalidIndexNameException.class, () -> resolveAbstractionsSelectorAllowed(List.of("*", "-*::custom")));
@@ -270,10 +238,6 @@ private List<String> resolveAbstractionsSelectorAllowed(List<String> expressions
270238
return resolveAbstractions(expressions, IndicesOptions.strictExpandOpen(), defaultMask);
271239
}
272240

273-
private List<String> resolveAbstractionsDefaultBothSelector(List<String> expressions) {
274-
return resolveAbstractions(expressions, IndicesOptions.strictExpandOpenIncludeFailureStore(), defaultMask);
275-
}
276-
277241
private List<String> resolveAbstractions(List<String> expressions, IndicesOptions indicesOptions, Supplier<Set<String>> mask) {
278242
return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx) -> true, true);
279243
}

0 commit comments

Comments
 (0)