Skip to content

Commit 1d6c6a5

Browse files
[Failure Store] Prevent usage of :: selectors with cross-cluster expressions (elastic#125252)
The CCS is currently not supported for failure store backing indices. This PR adjusts the selector parsing (introduced in elastic#118614) to prevent using `::failures` and `::data` selectors with cross-cluster expressions. For example, `GET my_remote_cluster:logs-*::failures/_search` request will fail early, during expression parsing. To test manually, run `./gradlew run-ccs` and execute the example request.
1 parent ea98166 commit 1d6c6a5

File tree

6 files changed

+1027
-9
lines changed

6 files changed

+1027
-9
lines changed

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2365,27 +2365,29 @@ private static <V> V splitSelectorExpression(String expression, BiFunction<Strin
23652365
int lastDoubleColon = expression.lastIndexOf(SELECTOR_SEPARATOR);
23662366
if (lastDoubleColon >= 0) {
23672367
String suffix = expression.substring(lastDoubleColon + SELECTOR_SEPARATOR.length());
2368-
doValidateSelectorString(() -> expression, suffix);
2368+
IndexComponentSelector selector = resolveAndValidateSelectorString(() -> expression, suffix);
23692369
String expressionBase = expression.substring(0, lastDoubleColon);
23702370
ensureNoMoreSelectorSeparators(expressionBase, expression);
2371+
ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(expressionBase, selector, expression);
23712372
return bindFunction.apply(expressionBase, suffix);
23722373
}
23732374
// Otherwise accept the default
23742375
return bindFunction.apply(expression, null);
23752376
}
23762377

23772378
public static void validateIndexSelectorString(String indexName, String suffix) {
2378-
doValidateSelectorString(() -> indexName + SELECTOR_SEPARATOR + suffix, suffix);
2379+
resolveAndValidateSelectorString(() -> indexName + SELECTOR_SEPARATOR + suffix, suffix);
23792380
}
23802381

2381-
private static void doValidateSelectorString(Supplier<String> expression, String suffix) {
2382+
private static IndexComponentSelector resolveAndValidateSelectorString(Supplier<String> expression, String suffix) {
23822383
IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix);
23832384
if (selector == null) {
23842385
throw new InvalidIndexNameException(
23852386
expression.get(),
23862387
"invalid usage of :: separator, [" + suffix + "] is not a recognized selector"
23872388
);
23882389
}
2390+
return selector;
23892391
}
23902392

23912393
/**
@@ -2416,6 +2418,22 @@ private static void ensureNoMoreSelectorSeparators(String remainingExpression, S
24162418
);
24172419
}
24182420
}
2421+
2422+
/**
2423+
* Checks the expression for remote cluster pattern and throws an exception if it is combined with :: selectors.
2424+
* @throws InvalidIndexNameException if remote cluster pattern is detected after parsing the selector expression
2425+
*/
2426+
private static void ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(
2427+
String expressionWithoutSelector,
2428+
IndexComponentSelector selector,
2429+
String originalExpression
2430+
) {
2431+
if (selector != null) {
2432+
if (RemoteClusterAware.isRemoteIndexName(expressionWithoutSelector)) {
2433+
throw new InvalidIndexNameException(originalExpression, "Selectors are not yet supported on remote cluster patterns");
2434+
}
2435+
}
2436+
}
24192437
}
24202438

24212439
/**

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

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@
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;
2325
import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
26+
import static org.hamcrest.Matchers.containsString;
2427
import static org.hamcrest.Matchers.equalTo;
2528
import static org.hamcrest.Matchers.is;
2629
import static org.hamcrest.Matchers.nullValue;
@@ -72,16 +75,49 @@ public void testResolveExpression() {
7275
// === Corner Cases
7376
// Empty index name is not necessarily disallowed, but will be filtered out in the next steps of resolution
7477
assertThat(resolve(selectorsAllowed, "::data"), equalTo(new ResolvedExpression("", DATA)));
75-
// Remote cluster syntax is respected, even if code higher up the call stack is likely to already have handled it already
76-
assertThat(resolve(selectorsAllowed, "cluster:index::data"), equalTo(new ResolvedExpression("cluster:index", DATA)));
77-
// CCS with an empty index name is not necessarily disallowed, though other code in the resolution logic will likely throw
78-
assertThat(resolve(selectorsAllowed, "cluster:::data"), equalTo(new ResolvedExpression("cluster:", DATA)));
79-
// Same for empty cluster and index names
78+
assertThat(resolve(selectorsAllowed, "::failures"), equalTo(new ResolvedExpression("", FAILURES)));
79+
// CCS with an empty index and cluster name is not necessarily disallowed, though other code in the resolution logic will likely
80+
// throw
8081
assertThat(resolve(selectorsAllowed, ":::data"), equalTo(new ResolvedExpression(":", DATA)));
82+
assertThat(resolve(selectorsAllowed, ":::failures"), equalTo(new ResolvedExpression(":", FAILURES)));
8183
// Any more prefix colon characters will trigger the multiple separators error logic
8284
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::data"));
85+
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::failures"));
86+
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, ":::::failures"));
8387
// Suffix case is not supported because there is no component named with the empty string
8488
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "index::"));
89+
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(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, expression));
119+
assertThat(e.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns"));
120+
}
85121
}
86122

87123
public void testResolveMatchAllToSelectors() {

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,13 @@ public void testInvalidCharacterInIndexPattern() {
637637
expectDoubleColonErrorWithLineNumber(command, "*:*::failures", parseLineNumber + 3);
638638

639639
// Too many colons
640-
expectInvalidIndexNameErrorWithLineNumber(command, "\"index:::data\"", lineNumber, "index:", "must not contain ':'");
640+
expectInvalidIndexNameErrorWithLineNumber(
641+
command,
642+
"\"index:::data\"",
643+
lineNumber,
644+
"index:::data",
645+
"Selectors are not yet supported on remote cluster patterns"
646+
);
641647
expectInvalidIndexNameErrorWithLineNumber(
642648
command,
643649
"\"index::::data\"",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.remotecluster;
9+
10+
import org.elasticsearch.action.search.SearchResponse;
11+
import org.elasticsearch.client.Request;
12+
import org.elasticsearch.client.RequestOptions;
13+
import org.elasticsearch.client.Response;
14+
import org.elasticsearch.client.ResponseException;
15+
import org.elasticsearch.common.xcontent.support.XContentMapValues;
16+
import org.elasticsearch.core.Tuple;
17+
import org.elasticsearch.search.SearchHit;
18+
import org.elasticsearch.search.SearchResponseUtils;
19+
20+
import java.io.IOException;
21+
import java.util.Arrays;
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.stream.Collectors;
25+
26+
import static org.hamcrest.Matchers.containsInAnyOrder;
27+
import static org.hamcrest.Matchers.containsString;
28+
import static org.hamcrest.Matchers.equalTo;
29+
30+
abstract class AbstractRemoteClusterSecurityFailureStoreRestIT extends AbstractRemoteClusterSecurityTestCase {
31+
32+
protected void assertSearchResponseContainsIndices(Response response, String... expectedIndices) throws IOException {
33+
assertOK(response);
34+
final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response));
35+
try {
36+
final List<String> actualIndices = Arrays.stream(searchResponse.getHits().getHits())
37+
.map(SearchHit::getIndex)
38+
.collect(Collectors.toList());
39+
assertThat(actualIndices, containsInAnyOrder(expectedIndices));
40+
} finally {
41+
searchResponse.decRef();
42+
}
43+
}
44+
45+
protected void setupTestDataStreamOnFulfillingCluster() throws IOException {
46+
// Create data stream and index some documents
47+
final Request createComponentTemplate = new Request("PUT", "/_component_template/component1");
48+
createComponentTemplate.setJsonEntity("""
49+
{
50+
"template": {
51+
"mappings": {
52+
"properties": {
53+
"@timestamp": {
54+
"type": "date"
55+
},
56+
"age": {
57+
"type": "integer"
58+
},
59+
"email": {
60+
"type": "keyword"
61+
},
62+
"name": {
63+
"type": "text"
64+
}
65+
}
66+
},
67+
"data_stream_options": {
68+
"failure_store": {
69+
"enabled": true
70+
}
71+
}
72+
}
73+
}""");
74+
assertOK(performRequestAgainstFulfillingCluster(createComponentTemplate));
75+
76+
final Request createTemplate = new Request("PUT", "/_index_template/template1");
77+
createTemplate.setJsonEntity("""
78+
{
79+
"index_patterns": ["test*"],
80+
"data_stream": {},
81+
"priority": 500,
82+
"composed_of": ["component1"]
83+
}""");
84+
assertOK(performRequestAgainstFulfillingCluster(createTemplate));
85+
86+
final Request createDoc1 = new Request("PUT", "/test1/_doc/1?refresh=true&op_type=create");
87+
createDoc1.setJsonEntity("""
88+
{
89+
"@timestamp": 1,
90+
"age" : 1,
91+
"name" : "jack",
92+
"email" : "[email protected]"
93+
}""");
94+
assertOK(performRequestAgainstFulfillingCluster(createDoc1));
95+
96+
final Request createDoc2 = new Request("PUT", "/test1/_doc/2?refresh=true&op_type=create");
97+
createDoc2.setJsonEntity("""
98+
{
99+
"@timestamp": 2,
100+
"age" : "this should be an int",
101+
"name" : "jack",
102+
"email" : "[email protected]"
103+
}""");
104+
assertOK(performRequestAgainstFulfillingCluster(createDoc2));
105+
{
106+
final Request otherTemplate = new Request("PUT", "/_index_template/other_template");
107+
otherTemplate.setJsonEntity("""
108+
{
109+
"index_patterns": ["other*"],
110+
"data_stream": {},
111+
"priority": 500,
112+
"composed_of": ["component1"]
113+
}""");
114+
assertOK(performRequestAgainstFulfillingCluster(otherTemplate));
115+
}
116+
{
117+
final Request createOtherDoc3 = new Request("PUT", "/other1/_doc/3?refresh=true&op_type=create");
118+
createOtherDoc3.setJsonEntity("""
119+
{
120+
"@timestamp": 3,
121+
"age" : 3,
122+
"name" : "jane",
123+
"email" : "[email protected]"
124+
}""");
125+
assertOK(performRequestAgainstFulfillingCluster(createOtherDoc3));
126+
}
127+
{
128+
final Request createOtherDoc4 = new Request("PUT", "/other1/_doc/4?refresh=true&op_type=create");
129+
createOtherDoc4.setJsonEntity("""
130+
{
131+
"@timestamp": 4,
132+
"age" : "this should be an int",
133+
"name" : "jane",
134+
"email" : "[email protected]"
135+
}""");
136+
assertOK(performRequestAgainstFulfillingCluster(createOtherDoc4));
137+
}
138+
}
139+
140+
protected Response performRequestWithRemoteSearchUser(final Request request) throws IOException {
141+
request.setOptions(
142+
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(REMOTE_SEARCH_USER, PASS))
143+
);
144+
return client().performRequest(request);
145+
}
146+
147+
protected Response performRequestWithUser(final String user, final Request request) throws IOException {
148+
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(user, PASS)));
149+
return client().performRequest(request);
150+
}
151+
152+
@SuppressWarnings("unchecked")
153+
protected Tuple<List<String>, List<String>> getDataAndFailureIndices(String dataStreamName) throws IOException {
154+
Request dataStream = new Request("GET", "/_data_stream/" + dataStreamName);
155+
Response response = performRequestAgainstFulfillingCluster(dataStream);
156+
Map<String, Object> dataStreams = entityAsMap(response);
157+
List<String> dataIndexNames = (List<String>) XContentMapValues.extractValue("data_streams.indices.index_name", dataStreams);
158+
List<String> failureIndexNames = (List<String>) XContentMapValues.extractValue(
159+
"data_streams.failure_store.indices.index_name",
160+
dataStreams
161+
);
162+
return new Tuple<>(dataIndexNames, failureIndexNames);
163+
}
164+
165+
protected Tuple<String, String> getSingleDataAndFailureIndices(String dataStreamName) throws IOException {
166+
Tuple<List<String>, List<String>> indices = getDataAndFailureIndices(dataStreamName);
167+
assertThat(indices.v1().size(), equalTo(1));
168+
assertThat(indices.v2().size(), equalTo(1));
169+
return new Tuple<>(indices.v1().get(0), indices.v2().get(0));
170+
}
171+
172+
protected static void assertSelectorsNotSupported(ResponseException exception) {
173+
assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403));
174+
assertThat(exception.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns"));
175+
}
176+
177+
}

0 commit comments

Comments
 (0)