Skip to content

Commit cacf00e

Browse files
slobodanadamovicslobodanadamovic
committed
[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. (cherry picked from commit 1d6c6a5) # Conflicts: # server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java
1 parent af4a548 commit cacf00e

File tree

5 files changed

+1032
-12
lines changed

5 files changed

+1032
-12
lines changed

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

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,21 +2126,31 @@ private static <V> V splitSelectorExpression(String expression, BiFunction<Strin
21262126
int lastDoubleColon = expression.lastIndexOf(SELECTOR_SEPARATOR);
21272127
if (lastDoubleColon >= 0) {
21282128
String suffix = expression.substring(lastDoubleColon + SELECTOR_SEPARATOR.length());
2129-
IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix);
2130-
if (selector == null) {
2131-
throw new InvalidIndexNameException(
2132-
expression,
2133-
"invalid usage of :: separator, [" + suffix + "] is not a recognized selector"
2134-
);
2135-
}
2129+
IndexComponentSelector selector = resolveAndValidateSelectorString(() -> expression, suffix);
21362130
String expressionBase = expression.substring(0, lastDoubleColon);
21372131
ensureNoMoreSelectorSeparators(expressionBase, expression);
2132+
ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(expressionBase, selector, expression);
21382133
return bindFunction.apply(expressionBase, suffix);
21392134
}
21402135
// Otherwise accept the default
21412136
return bindFunction.apply(expression, null);
21422137
}
21432138

2139+
public static void validateIndexSelectorString(String indexName, String suffix) {
2140+
resolveAndValidateSelectorString(() -> indexName + SELECTOR_SEPARATOR + suffix, suffix);
2141+
}
2142+
2143+
private static IndexComponentSelector resolveAndValidateSelectorString(Supplier<String> expression, String suffix) {
2144+
IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix);
2145+
if (selector == null) {
2146+
throw new InvalidIndexNameException(
2147+
expression.get(),
2148+
"invalid usage of :: separator, [" + suffix + "] is not a recognized selector"
2149+
);
2150+
}
2151+
return selector;
2152+
}
2153+
21442154
/**
21452155
* Checks the selectors that have been returned from splitting an expression and throws an exception if any were present.
21462156
* @param expression Original expression
@@ -2169,6 +2179,22 @@ private static void ensureNoMoreSelectorSeparators(String remainingExpression, S
21692179
);
21702180
}
21712181
}
2182+
2183+
/**
2184+
* Checks the expression for remote cluster pattern and throws an exception if it is combined with :: selectors.
2185+
* @throws InvalidIndexNameException if remote cluster pattern is detected after parsing the selector expression
2186+
*/
2187+
private static void ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(
2188+
String expressionWithoutSelector,
2189+
IndexComponentSelector selector,
2190+
String originalExpression
2191+
) {
2192+
if (selector != null) {
2193+
if (RemoteClusterAware.isRemoteIndexName(expressionWithoutSelector)) {
2194+
throw new InvalidIndexNameException(originalExpression, "Selectors are not yet supported on remote cluster patterns");
2195+
}
2196+
}
2197+
}
21722198
}
21732199

21742200
/**

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

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,13 @@
1818
import org.elasticsearch.indices.SystemIndices;
1919
import org.elasticsearch.test.ESTestCase;
2020

21+
import java.util.Set;
22+
2123
import static org.elasticsearch.action.support.IndexComponentSelector.DATA;
2224
import static org.elasticsearch.action.support.IndexComponentSelector.FAILURES;
2325
import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
2426
import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
27+
import static org.hamcrest.Matchers.containsString;
2528
import static org.hamcrest.Matchers.equalTo;
2629
import static org.hamcrest.Matchers.is;
2730
import static org.hamcrest.Matchers.nullValue;
@@ -73,16 +76,49 @@ public void testResolveExpression() {
7376
// === Corner Cases
7477
// Empty index name is not necessarily disallowed, but will be filtered out in the next steps of resolution
7578
assertThat(resolve(selectorsAllowed, "::data"), equalTo(new ResolvedExpression("", DATA)));
76-
// Remote cluster syntax is respected, even if code higher up the call stack is likely to already have handled it already
77-
assertThat(resolve(selectorsAllowed, "cluster:index::data"), equalTo(new ResolvedExpression("cluster:index", DATA)));
78-
// CCS with an empty index name is not necessarily disallowed, though other code in the resolution logic will likely throw
79-
assertThat(resolve(selectorsAllowed, "cluster:::data"), equalTo(new ResolvedExpression("cluster:", DATA)));
80-
// Same for empty cluster and index names
79+
assertThat(resolve(selectorsAllowed, "::failures"), equalTo(new ResolvedExpression("", FAILURES)));
80+
// CCS with an empty index and cluster name is not necessarily disallowed, though other code in the resolution logic will likely
81+
// throw
8182
assertThat(resolve(selectorsAllowed, ":::data"), equalTo(new ResolvedExpression(":", DATA)));
83+
assertThat(resolve(selectorsAllowed, ":::failures"), equalTo(new ResolvedExpression(":", FAILURES)));
8284
// Any more prefix colon characters will trigger the multiple separators error logic
8385
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::data"));
86+
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::failures"));
87+
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, ":::::failures"));
8488
// Suffix case is not supported because there is no component named with the empty string
8589
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "index::"));
90+
91+
// remote cluster syntax is not allowed with :: selectors
92+
final Set<String> remoteClusterExpressionsWithSelectors = Set.of(
93+
"cluster:index::failures",
94+
"cluster-*:index::failures",
95+
"cluster-*:index-*::failures",
96+
"cluster-*:*::failures",
97+
"*:index-*::failures",
98+
"*:*::failures",
99+
"*:-test*,*::failures",
100+
"cluster:::failures",
101+
"failures:index::failures",
102+
"data:index::failures",
103+
"failures:failures::failures",
104+
"data:data::failures",
105+
"cluster:index::data",
106+
"cluster-*:index::data",
107+
"cluster-*:index-*::data",
108+
"cluster-*:*::data",
109+
"*:index-*::data",
110+
"*:*::data",
111+
"cluster:::data",
112+
"failures:index::data",
113+
"data:index::data",
114+
"failures:failures::data",
115+
"data:data::data",
116+
"*:-test*,*::data"
117+
);
118+
for (String expression : remoteClusterExpressionsWithSelectors) {
119+
var e = expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, expression));
120+
assertThat(e.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns"));
121+
}
86122
}
87123

88124
public void testResolveMatchAllToSelectors() {

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

Lines changed: 177 additions & 0 deletions
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)