-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[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
slobodanadamovic
merged 34 commits into
elastic:main
from
slobodanadamovic:sa-prevent-using-selectors-for-ccs-ccr
Mar 28, 2025
Merged
Changes from 16 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 fc2271c
fail only if ::failures selector is used
slobodanadamovic ba976a9
cleanup and extend existing rest IT
slobodanadamovic f6c96e2
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic 5bc46b5
test ccs with rcs1
slobodanadamovic e92ffde
nit
slobodanadamovic 29de669
remove remote_indices - not relevant for RCS1 test case
slobodanadamovic eaf9a01
test CCS with RCS2
slobodanadamovic 41da2e6
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic 4e366ed
cleanup tests, handle edge cases with ccs_minimize_roundtrips
slobodanadamovic 542aa54
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic 816d919
more test users
slobodanadamovic 7dc89d4
fix assertion
slobodanadamovic 26e8ae3
test direct access to backing failure index for other users
slobodanadamovic 9c3d0d7
fix assertion
slobodanadamovic e8a4b96
fix another assertion
slobodanadamovic 6c8eee8
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic 6d5293f
bring back selector validation inline
slobodanadamovic fd1a0e5
refactor to reuse selector validation
slobodanadamovic c88a35f
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic 1bd03a6
more tests
slobodanadamovic 0dd4053
more tests 2
slobodanadamovic f085835
more test coverage
slobodanadamovic 446f8b6
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic 62664ac
[CI] Auto commit changes from spotless
0fe3ec5
prevent using ::data selector as well
slobodanadamovic ae8cc1a
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic 4baa39a
consolidate error messages and exceptions thrown
slobodanadamovic 5811200
fix failing test
slobodanadamovic bc0a5b5
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic 7d1dd3e
fix failing test ::data is not allowed
slobodanadamovic 5056909
Merge branch 'main' of github.com:elastic/elasticsearch into sa-preve…
slobodanadamovic 321d5c7
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic 148567d
Merge branch 'main' into sa-prevent-using-selectors-for-ccs-ccr
slobodanadamovic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
139 changes: 139 additions & 0 deletions
139
...rg/elasticsearch/xpack/remotecluster/AbstractRemoteClusterSecurityFailureStoreRestIT.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
/* | ||
* 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.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.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.stream.Collectors; | ||
|
||
import static org.hamcrest.Matchers.containsInAnyOrder; | ||
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)); | ||
} | ||
|
||
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); | ||
assertEquals(Collections.singletonList("test1"), XContentMapValues.extractValue("data_streams.name", dataStreams)); | ||
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)); | ||
} | ||
|
||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we simply fully bar any selector usage with cross cluster search? If the only supported option is
::data
, and no selector is the same as that, perhaps we should just disallow the syntax in case the suffix of::data
causes issues further down the line.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally suggested only restricting
::failures
but on second thought restricting::data
makes sense -- I guess it makes things simpler to just not allow any selectors at all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for raising this. If there is no real need to support
::data
it would make this more consistent and simpler. I'll make the change.