Skip to content

[Failure Store] Prevent usage of :: selectors with cross-cluster expressions #125252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2bc0e99
Prevent usage of :: selectors for remote cluster requests
slobodanadamovic Mar 19, 2025
fc2271c
fail only if ::failures selector is used
slobodanadamovic Mar 19, 2025
ba976a9
cleanup and extend existing rest IT
slobodanadamovic Mar 19, 2025
f6c96e2
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic Mar 19, 2025
5bc46b5
test ccs with rcs1
slobodanadamovic Mar 20, 2025
e92ffde
nit
slobodanadamovic Mar 20, 2025
29de669
remove remote_indices - not relevant for RCS1 test case
slobodanadamovic Mar 20, 2025
eaf9a01
test CCS with RCS2
slobodanadamovic Mar 20, 2025
41da2e6
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic Mar 20, 2025
4e366ed
cleanup tests, handle edge cases with ccs_minimize_roundtrips
slobodanadamovic Mar 21, 2025
542aa54
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic Mar 21, 2025
816d919
more test users
slobodanadamovic Mar 21, 2025
7dc89d4
fix assertion
slobodanadamovic Mar 21, 2025
26e8ae3
test direct access to backing failure index for other users
slobodanadamovic Mar 21, 2025
9c3d0d7
fix assertion
slobodanadamovic Mar 21, 2025
e8a4b96
fix another assertion
slobodanadamovic Mar 21, 2025
6c8eee8
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic Mar 25, 2025
6d5293f
bring back selector validation inline
slobodanadamovic Mar 25, 2025
fd1a0e5
refactor to reuse selector validation
slobodanadamovic Mar 25, 2025
c88a35f
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic Mar 25, 2025
1bd03a6
more tests
slobodanadamovic Mar 25, 2025
0dd4053
more tests 2
slobodanadamovic Mar 25, 2025
f085835
more test coverage
slobodanadamovic Mar 25, 2025
446f8b6
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic Mar 25, 2025
62664ac
[CI] Auto commit changes from spotless
elasticsearchmachine Mar 25, 2025
0fe3ec5
prevent using ::data selector as well
slobodanadamovic Mar 26, 2025
ae8cc1a
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic Mar 26, 2025
4baa39a
consolidate error messages and exceptions thrown
slobodanadamovic Mar 26, 2025
5811200
fix failing test
slobodanadamovic Mar 26, 2025
bc0a5b5
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic Mar 26, 2025
7d1dd3e
fix failing test ::data is not allowed
slobodanadamovic Mar 27, 2025
5056909
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic Mar 27, 2025
321d5c7
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic Mar 27, 2025
148567d
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic Mar 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2365,27 +2365,29 @@ private static <V> V splitSelectorExpression(String expression, BiFunction<Strin
int lastDoubleColon = expression.lastIndexOf(SELECTOR_SEPARATOR);
if (lastDoubleColon >= 0) {
String suffix = expression.substring(lastDoubleColon + SELECTOR_SEPARATOR.length());
doValidateSelectorString(() -> expression, suffix);
IndexComponentSelector selector = resolveAndValidateSelectorString(() -> expression, suffix);
String expressionBase = expression.substring(0, lastDoubleColon);
ensureNoMoreSelectorSeparators(expressionBase, expression);
ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(expressionBase, selector, expression);
return bindFunction.apply(expressionBase, suffix);
}
// Otherwise accept the default
return bindFunction.apply(expression, null);
}

public static void validateIndexSelectorString(String indexName, String suffix) {
doValidateSelectorString(() -> indexName + SELECTOR_SEPARATOR + suffix, suffix);
resolveAndValidateSelectorString(() -> indexName + SELECTOR_SEPARATOR + suffix, suffix);
}

private static void doValidateSelectorString(Supplier<String> expression, String suffix) {
private static IndexComponentSelector resolveAndValidateSelectorString(Supplier<String> expression, String suffix) {
IndexComponentSelector selector = IndexComponentSelector.getByKey(suffix);
if (selector == null) {
throw new InvalidIndexNameException(
expression.get(),
"invalid usage of :: separator, [" + suffix + "] is not a recognized selector"
);
}
return selector;
}

/**
Expand Down Expand Up @@ -2416,6 +2418,22 @@ private static void ensureNoMoreSelectorSeparators(String remainingExpression, S
);
}
}

/**
* Checks the expression for remote cluster pattern and throws an exception if it is combined with :: selectors.
* @throws InvalidIndexNameException if remote cluster pattern is detected after parsing the selector expression
*/
private static void ensureNotMixingRemoteClusterExpressionWithSelectorSeparator(
String expressionWithoutSelector,
IndexComponentSelector selector,
String originalExpression
) {
if (selector != null) {
if (RemoteClusterAware.isRemoteIndexName(expressionWithoutSelector)) {
throw new InvalidIndexNameException(originalExpression, "Selectors are not yet supported on remote cluster patterns");
}
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@
import org.elasticsearch.indices.SystemIndices;
import org.elasticsearch.test.ESTestCase;

import java.util.Set;

import static org.elasticsearch.action.support.IndexComponentSelector.DATA;
import static org.elasticsearch.action.support.IndexComponentSelector.FAILURES;
import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.ResolvedExpression;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
Expand Down Expand Up @@ -72,16 +75,49 @@ public void testResolveExpression() {
// === Corner Cases
// Empty index name is not necessarily disallowed, but will be filtered out in the next steps of resolution
assertThat(resolve(selectorsAllowed, "::data"), equalTo(new ResolvedExpression("", DATA)));
// Remote cluster syntax is respected, even if code higher up the call stack is likely to already have handled it already
assertThat(resolve(selectorsAllowed, "cluster:index::data"), equalTo(new ResolvedExpression("cluster:index", DATA)));
// CCS with an empty index name is not necessarily disallowed, though other code in the resolution logic will likely throw
assertThat(resolve(selectorsAllowed, "cluster:::data"), equalTo(new ResolvedExpression("cluster:", DATA)));
// Same for empty cluster and index names
assertThat(resolve(selectorsAllowed, "::failures"), equalTo(new ResolvedExpression("", FAILURES)));
// CCS with an empty index and cluster name is not necessarily disallowed, though other code in the resolution logic will likely
// throw
assertThat(resolve(selectorsAllowed, ":::data"), equalTo(new ResolvedExpression(":", DATA)));
assertThat(resolve(selectorsAllowed, ":::failures"), equalTo(new ResolvedExpression(":", FAILURES)));
// Any more prefix colon characters will trigger the multiple separators error logic
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::data"));
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "::::failures"));
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, ":::::failures"));
// Suffix case is not supported because there is no component named with the empty string
expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, "index::"));

// remote cluster syntax is not allowed with :: selectors
final Set<String> remoteClusterExpressionsWithSelectors = Set.of(
"cluster:index::failures",
"cluster-*:index::failures",
"cluster-*:index-*::failures",
"cluster-*:*::failures",
"*:index-*::failures",
"*:*::failures",
"*:-test*,*::failures",
"cluster:::failures",
"failures:index::failures",
"data:index::failures",
"failures:failures::failures",
"data:data::failures",
"cluster:index::data",
"cluster-*:index::data",
"cluster-*:index-*::data",
"cluster-*:*::data",
"*:index-*::data",
"*:*::data",
"cluster:::data",
"failures:index::data",
"data:index::data",
"failures:failures::data",
"data:data::data",
"*:-test*,*::data"
);
for (String expression : remoteClusterExpressionsWithSelectors) {
var e = expectThrows(InvalidIndexNameException.class, () -> resolve(selectorsAllowed, expression));
assertThat(e.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns"));
}
}

public void testResolveMatchAllToSelectors() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,13 @@ public void testInvalidCharacterInIndexPattern() {
expectDoubleColonErrorWithLineNumber(command, "*:*::failures", parseLineNumber + 3);

// Too many colons
expectInvalidIndexNameErrorWithLineNumber(command, "\"index:::data\"", lineNumber, "index:", "must not contain ':'");
expectInvalidIndexNameErrorWithLineNumber(
command,
"\"index:::data\"",
lineNumber,
"index:::data",
"Selectors are not yet supported on remote cluster patterns"
);
expectInvalidIndexNameErrorWithLineNumber(
command,
"\"index::::data\"",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.remotecluster;

import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.RequestOptions;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchResponseUtils;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;

abstract class AbstractRemoteClusterSecurityFailureStoreRestIT extends AbstractRemoteClusterSecurityTestCase {

protected void assertSearchResponseContainsIndices(Response response, String... expectedIndices) throws IOException {
assertOK(response);
final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response));
try {
final List<String> actualIndices = Arrays.stream(searchResponse.getHits().getHits())
.map(SearchHit::getIndex)
.collect(Collectors.toList());
assertThat(actualIndices, containsInAnyOrder(expectedIndices));
} finally {
searchResponse.decRef();
}
}

protected void setupTestDataStreamOnFulfillingCluster() throws IOException {
// Create data stream and index some documents
final Request createComponentTemplate = new Request("PUT", "/_component_template/component1");
createComponentTemplate.setJsonEntity("""
{
"template": {
"mappings": {
"properties": {
"@timestamp": {
"type": "date"
},
"age": {
"type": "integer"
},
"email": {
"type": "keyword"
},
"name": {
"type": "text"
}
}
},
"data_stream_options": {
"failure_store": {
"enabled": true
}
}
}
}""");
assertOK(performRequestAgainstFulfillingCluster(createComponentTemplate));

final Request createTemplate = new Request("PUT", "/_index_template/template1");
createTemplate.setJsonEntity("""
{
"index_patterns": ["test*"],
"data_stream": {},
"priority": 500,
"composed_of": ["component1"]
}""");
assertOK(performRequestAgainstFulfillingCluster(createTemplate));

final Request createDoc1 = new Request("PUT", "/test1/_doc/1?refresh=true&op_type=create");
createDoc1.setJsonEntity("""
{
"@timestamp": 1,
"age" : 1,
"name" : "jack",
"email" : "[email protected]"
}""");
assertOK(performRequestAgainstFulfillingCluster(createDoc1));

final Request createDoc2 = new Request("PUT", "/test1/_doc/2?refresh=true&op_type=create");
createDoc2.setJsonEntity("""
{
"@timestamp": 2,
"age" : "this should be an int",
"name" : "jack",
"email" : "[email protected]"
}""");
assertOK(performRequestAgainstFulfillingCluster(createDoc2));
{
final Request otherTemplate = new Request("PUT", "/_index_template/other_template");
otherTemplate.setJsonEntity("""
{
"index_patterns": ["other*"],
"data_stream": {},
"priority": 500,
"composed_of": ["component1"]
}""");
assertOK(performRequestAgainstFulfillingCluster(otherTemplate));
}
{
final Request createOtherDoc3 = new Request("PUT", "/other1/_doc/3?refresh=true&op_type=create");
createOtherDoc3.setJsonEntity("""
{
"@timestamp": 3,
"age" : 3,
"name" : "jane",
"email" : "[email protected]"
}""");
assertOK(performRequestAgainstFulfillingCluster(createOtherDoc3));
}
{
final Request createOtherDoc4 = new Request("PUT", "/other1/_doc/4?refresh=true&op_type=create");
createOtherDoc4.setJsonEntity("""
{
"@timestamp": 4,
"age" : "this should be an int",
"name" : "jane",
"email" : "[email protected]"
}""");
assertOK(performRequestAgainstFulfillingCluster(createOtherDoc4));
}
}

protected Response performRequestWithRemoteSearchUser(final Request request) throws IOException {
request.setOptions(
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(REMOTE_SEARCH_USER, PASS))
);
return client().performRequest(request);
}

protected Response performRequestWithUser(final String user, final Request request) throws IOException {
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", headerFromRandomAuthMethod(user, PASS)));
return client().performRequest(request);
}

@SuppressWarnings("unchecked")
protected Tuple<List<String>, List<String>> getDataAndFailureIndices(String dataStreamName) throws IOException {
Request dataStream = new Request("GET", "/_data_stream/" + dataStreamName);
Response response = performRequestAgainstFulfillingCluster(dataStream);
Map<String, Object> dataStreams = entityAsMap(response);
List<String> dataIndexNames = (List<String>) XContentMapValues.extractValue("data_streams.indices.index_name", dataStreams);
List<String> failureIndexNames = (List<String>) XContentMapValues.extractValue(
"data_streams.failure_store.indices.index_name",
dataStreams
);
return new Tuple<>(dataIndexNames, failureIndexNames);
}

protected Tuple<String, String> getSingleDataAndFailureIndices(String dataStreamName) throws IOException {
Tuple<List<String>, List<String>> indices = getDataAndFailureIndices(dataStreamName);
assertThat(indices.v1().size(), equalTo(1));
assertThat(indices.v2().size(), equalTo(1));
return new Tuple<>(indices.v1().get(0), indices.v2().get(0));
}

protected static void assertSelectorsNotSupported(ResponseException exception) {
assertThat(exception.getResponse().getStatusLine().getStatusCode(), equalTo(403));
assertThat(exception.getMessage(), containsString("Selectors are not yet supported on remote cluster patterns"));
}

}
Loading