Skip to content

Commit 0fe3ec5

Browse files
slobodanadamovicslobodanadamovic
committed
prevent using ::data selector as well
1 parent 62664ac commit 0fe3ec5

File tree

5 files changed

+92
-40
lines changed

5 files changed

+92
-40
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,7 +2368,7 @@ private static <V> V splitSelectorExpression(String expression, BiFunction<Strin
23682368
IndexComponentSelector selector = resolveAndValidateSelectorString(() -> expression, suffix);
23692369
String expressionBase = expression.substring(0, lastDoubleColon);
23702370
ensureNoMoreSelectorSeparators(expressionBase, expression);
2371-
ensureNoCrossClusterExpressionWithFailuresSelector(expressionBase, selector, expression);
2371+
ensureNoCrossClusterExpressionWithSelectorSeparator(expressionBase, selector, expression);
23722372
return bindFunction.apply(expressionBase, suffix);
23732373
}
23742374
// Otherwise accept the default
@@ -2423,20 +2423,20 @@ private static void ensureNoMoreSelectorSeparators(String remainingExpression, S
24232423
* Checks the expression for cross-cluster syntax and throws an exception if it is combined with ::failures selector.
24242424
* @throws IllegalArgumentException if cross-cluster syntax is detected after parsing the selector expression
24252425
*/
2426-
private static void ensureNoCrossClusterExpressionWithFailuresSelector(
2426+
private static void ensureNoCrossClusterExpressionWithSelectorSeparator(
24272427
String expressionWithoutSelector,
24282428
IndexComponentSelector selector,
24292429
String originalExpression
24302430
) {
2431-
if (selector == IndexComponentSelector.FAILURES) {
2431+
if (selector != null) {
24322432
if (RemoteClusterAware.isRemoteIndexName(expressionWithoutSelector)) {
24332433
throw new IllegalArgumentException(
24342434
"Invalid usage of ["
24352435
+ SELECTOR_SEPARATOR
24362436
+ selector.getKey()
24372437
+ "] selector in ["
24382438
+ originalExpression
2439-
+ "], failures selector is not supported with cross-cluster expressions"
2439+
+ "], selectors are not supported with cross-cluster expressions"
24402440
);
24412441
}
24422442
}

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

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
import org.elasticsearch.indices.SystemIndices;
1818
import org.elasticsearch.test.ESTestCase;
1919

20+
import java.util.Set;
21+
2022
import static org.elasticsearch.action.support.IndexComponentSelector.DATA;
2123
import static org.elasticsearch.action.support.IndexComponentSelector.FAILURES;
2224
import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
@@ -74,11 +76,8 @@ public void testResolveExpression() {
7476
// Empty index name is not necessarily disallowed, but will be filtered out in the next steps of resolution
7577
assertThat(resolve(selectorsAllowed, "::data"), equalTo(new ResolvedExpression("", DATA)));
7678
assertThat(resolve(selectorsAllowed, "::failures"), equalTo(new ResolvedExpression("", FAILURES)));
77-
// Remote cluster syntax is respected, even if code higher up the call stack is likely to already have handled it already
78-
assertThat(resolve(selectorsAllowed, "cluster:index::data"), equalTo(new ResolvedExpression("cluster:index", DATA)));
79-
// CCS with an empty index name is not necessarily disallowed, though other code in the resolution logic will likely throw
80-
assertThat(resolve(selectorsAllowed, "cluster:::data"), equalTo(new ResolvedExpression("cluster:", DATA)));
81-
// Same for empty cluster and index names
79+
// CCS with an empty index and cluster name is not necessarily disallowed, though other code in the resolution logic will likely
80+
// throw
8281
assertThat(resolve(selectorsAllowed, ":::data"), equalTo(new ResolvedExpression(":", DATA)));
8382
assertThat(resolve(selectorsAllowed, ":::failures"), equalTo(new ResolvedExpression(":", FAILURES)));
8483
// Any more prefix colon characters will trigger the multiple separators error logic
@@ -88,25 +87,37 @@ public void testResolveExpression() {
8887
// Suffix case is not supported because there is no component named with the empty string
8988
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "index::"));
9089

91-
// remote cluster syntax is not allowed with ::failures selector
92-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:index::failures");
93-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(noSelectors, "cluster:index::failures");
94-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:index::failures");
95-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:index-*::failures");
96-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster-*:*::failures");
97-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:index-*::failures");
98-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "*:*::failures");
99-
// even with an empty index name
100-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "cluster:::failures");
101-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "failures:index::failures");
102-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "data:index::failures");
103-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "failures:failures::failures");
104-
assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(selectorsAllowed, "data:data::failures");
105-
}
106-
107-
public void assertFailuresSelectorNotSupportedWithRemoteClusterExpressions(Context context, String expression) {
108-
var e = expectThrows(IllegalArgumentException.class, () -> resolve(context, expression));
109-
assertThat(e.getMessage(), containsString("failures selector is not supported with cross-cluster expressions"));
90+
// remote cluster syntax is not allowed with :: selectors
91+
final Set<String> remoteClusterExpressionsWithSelectors = Set.of(
92+
"cluster:index::failures",
93+
"cluster-*:index::failures",
94+
"cluster-*:index-*::failures",
95+
"cluster-*:*::failures",
96+
"*:index-*::failures",
97+
"*:*::failures",
98+
"*:-test*,*::failures",
99+
"cluster:::failures",
100+
"failures:index::failures",
101+
"data:index::failures",
102+
"failures:failures::failures",
103+
"data:data::failures",
104+
"cluster:index::data",
105+
"cluster-*:index::data",
106+
"cluster-*:index-*::data",
107+
"cluster-*:*::data",
108+
"*:index-*::data",
109+
"*:*::data",
110+
"cluster:::data",
111+
"failures:index::data",
112+
"data:index::data",
113+
"failures:failures::data",
114+
"data:data::data",
115+
"*:-test*,*::data"
116+
);
117+
for (String expression : remoteClusterExpressionsWithSelectors) {
118+
var e = expectThrows(IllegalArgumentException.class, () -> resolve(selectorsAllowed, expression));
119+
assertThat(e.getMessage(), containsString("selectors are not supported with cross-cluster expressions"));
120+
}
110121
}
111122

112123
public void testResolveMatchAllToSelectors() {

‎x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,9 @@ protected Tuple<String, String> getSingleDataAndFailureIndices(String dataStream
169169
return new Tuple<>(indices.v1().get(0), indices.v2().get(0));
170170
}
171171

172-
protected static void assertFailuresSelectorNotSupported(ResponseException exception) {
172+
protected static void assertSelectorsNotSupported(ResponseException exception) {
173173
assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403));
174-
assertThat(exception.getMessage(), containsString("failures selector is not supported with cross-cluster expressions"));
174+
assertThat(exception.getMessage(), containsString("selectors are not supported with cross-cluster expressions"));
175175
}
176176

177177
}

‎x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS1FailureStoreRestIT.java‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ public void testRCS1CrossClusterSearch() throws Exception {
8383
final String otherBackingDataIndexName = otherBackingIndices.v1();
8484
final String otherBackingFailureIndexName = otherBackingIndices.v2();
8585

86-
testCcsWithDataSelectorSupported(backingDataIndexName, ccsMinimizeRoundtrips);
86+
testCcsWithDataSelectorNotSupported(ccsMinimizeRoundtrips);
8787
testCcsWithFailuresSelectorNotSupported(ccsMinimizeRoundtrips);
88-
88+
testCcsWithoutSelectorsSupported(backingDataIndexName, ccsMinimizeRoundtrips);
8989
testSearchingUnauthorizedIndices(otherBackingFailureIndexName, otherBackingDataIndexName, ccsMinimizeRoundtrips);
9090
testSearchingWithAccessToAllIndices(ccsMinimizeRoundtrips, backingDataIndexName, otherBackingDataIndexName);
9191
testBackingFailureIndexAccess(ccsMinimizeRoundtrips, backingFailureIndexName);
@@ -264,10 +264,9 @@ private void testBackingFailureIndexAccess(boolean ccsMinimizeRoundtrips, String
264264

265265
}
266266

267-
private void testCcsWithDataSelectorSupported(String backingDataIndexName, boolean ccsMinimizeRoundtrips) throws IOException {
267+
public void testCcsWithoutSelectorsSupported(String backingDataIndexName, boolean ccsMinimizeRoundtrips) throws IOException {
268268
final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS };
269269
for (String user : users) {
270-
// query remote cluster using ::data selector should succeed
271270
final boolean alsoSearchLocally = randomBoolean();
272271
final Request dataSearchRequest = new Request(
273272
"GET",
@@ -276,7 +275,7 @@ private void testCcsWithDataSelectorSupported(String backingDataIndexName, boole
276275
"/%s%s:%s/_search?ccs_minimize_roundtrips=%s",
277276
alsoSearchLocally ? "local_index," : "",
278277
randomFrom("my_remote_cluster", "*", "my_remote_*"),
279-
randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName),
278+
randomFrom("test1", "test*", "*", backingDataIndexName),
280279
ccsMinimizeRoundtrips
281280
)
282281
);
@@ -287,6 +286,29 @@ private void testCcsWithDataSelectorSupported(String backingDataIndexName, boole
287286
}
288287
}
289288

289+
private void testCcsWithDataSelectorNotSupported(boolean ccsMinimizeRoundtrips) throws IOException {
290+
final String[] users = { FAILURE_STORE_ACCESS, DATA_ACCESS, ALL_ACCESS };
291+
for (String user : users) {
292+
// query remote cluster using ::data selector should not succeed
293+
final boolean alsoSearchLocally = randomBoolean();
294+
final Request dataSearchRequest = new Request(
295+
"GET",
296+
String.format(
297+
Locale.ROOT,
298+
"/%s:%s/_search?ccs_minimize_roundtrips=%s",
299+
randomFrom("my_remote_cluster", "*", "my_remote_*"),
300+
randomFrom("test1::data", "test*::data", "*::data", "non-existing::data"),
301+
ccsMinimizeRoundtrips
302+
)
303+
);
304+
final ResponseException exception = expectThrows(
305+
ResponseException.class,
306+
() -> performRequestWithUser(user, dataSearchRequest)
307+
);
308+
assertSelectorsNotSupported(exception);
309+
}
310+
}
311+
290312
private void testCcsWithFailuresSelectorNotSupported(boolean ccsMinimizeRoundtrips) {
291313
final String[] users = {
292314
FAILURE_STORE_ACCESS,
@@ -313,7 +335,7 @@ private void testCcsWithFailuresSelectorNotSupported(boolean ccsMinimizeRoundtri
313335
)
314336
)
315337
);
316-
assertFailuresSelectorNotSupported(exception);
338+
assertSelectorsNotSupported(exception);
317339
}
318340
}
319341

‎x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityRCS2FailureStoreRestIT.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public void testRCS2CrossClusterSearch() throws Exception {
8989
final String backingDataIndexName = backingIndices.v1();
9090
final String backingFailureIndexName = backingIndices.v2();
9191
{
92-
// query remote cluster using ::data selector should succeed
92+
// query remote cluster without selectors should succeed
9393
final boolean alsoSearchLocally = randomBoolean();
9494
final Request dataSearchRequest = new Request(
9595
"GET",
@@ -98,7 +98,7 @@ public void testRCS2CrossClusterSearch() throws Exception {
9898
"/%s%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false",
9999
alsoSearchLocally ? "local_index," : "",
100100
randomFrom("my_remote_cluster", "*", "my_remote_*"),
101-
randomFrom("test1::data", "test1", "test*", "test*::data", "*", "*::data", backingDataIndexName),
101+
randomFrom("test1", "test*", "*", backingDataIndexName),
102102
ccsMinimizeRoundtrips
103103
)
104104
);
@@ -107,6 +107,25 @@ public void testRCS2CrossClusterSearch() throws Exception {
107107
: new String[] { backingDataIndexName };
108108
assertSearchResponseContainsIndices(performRequestWithRemoteSearchUser(dataSearchRequest), expectedIndices);
109109
}
110+
{
111+
// query remote cluster using ::data selector should fail
112+
final boolean alsoSearchLocally = randomBoolean();
113+
final Request dataSearchRequest = new Request(
114+
"GET",
115+
String.format(
116+
Locale.ROOT,
117+
"/%s:%s/_search?ccs_minimize_roundtrips=%s&ignore_unavailable=false",
118+
randomFrom("my_remote_cluster", "*", "my_remote_*"),
119+
randomFrom("test1::data", "test*::data", "*::data", "non-existing::data"),
120+
ccsMinimizeRoundtrips
121+
)
122+
);
123+
final ResponseException exception = expectThrows(
124+
ResponseException.class,
125+
() -> performRequestWithRemoteSearchUser(dataSearchRequest)
126+
);
127+
assertSelectorsNotSupported(exception);
128+
}
110129
{
111130
// query remote cluster using ::failures selector should fail
112131
final ResponseException exception = expectThrows(
@@ -117,13 +136,13 @@ public void testRCS2CrossClusterSearch() throws Exception {
117136
String.format(
118137
Locale.ROOT,
119138
"/my_remote_cluster:%s/_search?ccs_minimize_roundtrips=%s",
120-
randomFrom("test1::failures", "test*::failures", "*::failures"),
139+
randomFrom("test1::failures", "test*::failures", "*::failures", "non-existing::failures"),
121140
ccsMinimizeRoundtrips
122141
)
123142
)
124143
)
125144
);
126-
assertFailuresSelectorNotSupported(exception);
145+
assertSelectorsNotSupported(exception);
127146
}
128147
{
129148
// direct access to backing failure index is not allowed - no explicit read privileges over .fs-* indices

0 commit comments

Comments
 (0)