Skip to content
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

Improve shard level request cache efficiency (#69505) #69515

Merged
merged 1 commit into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -66,7 +66,7 @@ public class ShardSearchRequest extends TransportRequest implements IndicesReque
private final Scroll scroll;
private final String[] types;
private final float indexBoost;
private final Boolean requestCache;
private Boolean requestCache;
private final long nowInMillis;
private final boolean allowPartialSearchResults;
private final OriginalIndices originalIndices;
Expand Down Expand Up @@ -344,6 +344,10 @@ public Boolean requestCache() {
return requestCache;
}

public void requestCache(Boolean requestCache) {
this.requestCache = requestCache;
}

public boolean allowPartialSearchResults() {
return allowPartialSearchResults;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ protected String configUsers() {
"user1:" + usersPasswdHashed + "\n" +
"user2:" + usersPasswdHashed + "\n" +
"user3:" + usersPasswdHashed + "\n" +
"user4:" + usersPasswdHashed + "\n";
"user4:" + usersPasswdHashed + "\n" +
"user5:" + usersPasswdHashed + "\n";
}

@Override
Expand All @@ -138,7 +139,8 @@ protected String configUsersRoles() {
"role1:user1,user2,user3\n" +
"role2:user1,user3\n" +
"role3:user2,user3\n" +
"role4:user4\n";
"role4:user4\n" +
"role5:user5\n";
}

@Override
Expand Down Expand Up @@ -171,7 +173,17 @@ protected String configRoles() {
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
// query that can match nested documents
" query: '{\"bool\": { \"must_not\": { \"term\" : {\"field1\" : \"value2\"}}}}'";
" query: '{\"bool\": { \"must_not\": { \"term\" : {\"field1\" : \"value2\"}}}}'\n" +
"role5:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: [ 'test' ]\n" +
" privileges: [ read ]\n" +
" query: '{\"term\" : {\"field2\" : \"value2\"}}'\n" +
" - names: [ 'fls-index' ]\n" +
" privileges: [ read ]\n" +
" field_security:\n" +
" grant: [ 'field1', 'other_field', 'suggest_field2' ]\n";
}

@Override
Expand Down Expand Up @@ -1278,6 +1290,15 @@ public void testSuggesters() throws Exception {
.endObject()).get();
refresh("test");

assertAcked(client().admin().indices().prepareCreate("fls-index")
.setSettings(Settings.builder()
.put("index.number_of_shards", 1)
.put("index.number_of_replicas", 0)
)
.addMapping("type1", "field1", "type=text", "suggest_field1", "type=text", "suggest_field2", "type=completion",
"yet_another", "type=text")
);

// Term suggester:
SearchResponse response = client()
.prepareSearch("test")
Expand All @@ -1293,9 +1314,13 @@ public void testSuggesters() throws Exception {
assertThat(termSuggestion.getEntries().get(0).getOptions().size(), equalTo(1));
assertThat(termSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));

final String[] indices =
randomFrom(org.elasticsearch.common.collect.List.of(
new String[] { "test" }, new String[] { "fls-index", "test" }, new String[] { "test", "fls-index" }));

Exception e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch(indices)
.suggest(new SuggestBuilder()
.setGlobalText("valeu")
.addSuggestion("_name1", new TermSuggestionBuilder("suggest_field1"))
Expand All @@ -1318,8 +1343,8 @@ public void testSuggesters() throws Exception {
assertThat(phraseSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));

e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch(indices)
.suggest(new SuggestBuilder()
.setGlobalText("valeu")
.addSuggestion("_name1", new PhraseSuggestionBuilder("suggest_field1"))
Expand All @@ -1342,8 +1367,8 @@ public void testSuggesters() throws Exception {
assertThat(completionSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));

e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch(indices)
.suggest(new SuggestBuilder()
.setGlobalText("valeu")
.addSuggestion("_name1", new CompletionSuggestionBuilder("suggest_field2"))
Expand Down Expand Up @@ -1373,6 +1398,14 @@ public void testProfile() throws Exception {
.endObject()).get();
refresh("test");

assertAcked(client().admin().indices().prepareCreate("fls-index")
.setSettings(Settings.builder()
.put("index.number_of_shards", 1)
.put("index.number_of_replicas", 0)
)
.addMapping("type1", "field1", "type=text", "other_field", "type=text", "yet_another", "type=text")
);

SearchResponse response = client()
.prepareSearch("test")
.setProfile(true)
Expand All @@ -1389,9 +1422,12 @@ public void testProfile() throws Exception {
// ProfileResult profileResult = queryProfileShardResult.getQueryResults().get(0);
// assertThat(profileResult.getLuceneDescription(), equalTo("(other_field:value)^0.8"));

final String[] indices =
randomFrom(org.elasticsearch.common.collect.List.of(
new String[] { "test" }, new String[] { "fls-index", "test" }, new String[] { "test", "fls-index" }));
Exception e = expectThrows(ElasticsearchSecurityException.class, () -> client()
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
.prepareSearch("test")
.filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
.prepareSearch(indices)
.setProfile(true)
.setQuery(new FuzzyQueryBuilder("other_field", "valeu"))
.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.IndicesRequest;
import org.elasticsearch.common.MemoizedSupplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.XPackLicenseState.Feature;
import org.elasticsearch.transport.TransportActionProxy;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.RequestInfo;
Expand All @@ -39,28 +41,37 @@ abstract class FieldAndDocumentLevelSecurityRequestInterceptor implements Reques
@Override
public void intercept(RequestInfo requestInfo, AuthorizationEngine authorizationEngine, AuthorizationInfo authorizationInfo,
ActionListener<Void> listener) {
if (requestInfo.getRequest() instanceof IndicesRequest) {
if (requestInfo.getRequest() instanceof IndicesRequest && false == TransportActionProxy.isProxyAction(requestInfo.getAction())) {
IndicesRequest indicesRequest = (IndicesRequest) requestInfo.getRequest();
boolean shouldIntercept = licenseState.isSecurityEnabled();
MemoizedSupplier<Boolean> licenseChecker = new MemoizedSupplier<>(() -> licenseState.checkFeature(Feature.SECURITY_DLS_FLS));
if (supports(indicesRequest) && shouldIntercept) {
final IndicesAccessControl indicesAccessControl =
threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
for (String index : requestIndices(indicesRequest)) {
boolean fieldLevelSecurityEnabled = false;
boolean documentLevelSecurityEnabled = false;
final String[] requestIndices = requestIndices(indicesRequest);
for (String index : requestIndices) {
IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(index);
if (indexAccessControl != null) {
boolean fieldLevelSecurityEnabled = indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
boolean documentLevelSecurityEnabled = indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
if ((fieldLevelSecurityEnabled || documentLevelSecurityEnabled) && licenseChecker.get()) {
logger.trace("intercepted request for index [{}] with field level access controls [{}] " +
"document level access controls [{}]. disabling conflicting features",
index, fieldLevelSecurityEnabled, documentLevelSecurityEnabled);
disableFeatures(indicesRequest, fieldLevelSecurityEnabled, documentLevelSecurityEnabled, listener);
return;
fieldLevelSecurityEnabled =
fieldLevelSecurityEnabled || indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
documentLevelSecurityEnabled =
documentLevelSecurityEnabled || indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
if (fieldLevelSecurityEnabled && documentLevelSecurityEnabled) {
break;
}
}
logger.trace("intercepted request for index [{}] without field or document level access controls", index);
}
if ((fieldLevelSecurityEnabled || documentLevelSecurityEnabled) && licenseChecker.get()) {
logger.trace("intercepted request for indices [{}] with field level access controls [{}] " +
"document level access controls [{}]. disabling conflicting features",
Strings.arrayToDelimitedString(requestIndices, ","), fieldLevelSecurityEnabled, documentLevelSecurityEnabled);
disableFeatures(indicesRequest, fieldLevelSecurityEnabled, documentLevelSecurityEnabled, listener);
return;
}
logger.trace("intercepted request for indices [{}] without field or document level access controls",
Strings.arrayToDelimitedString(requestIndices, ","));
}
}
listener.onResponse(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.internal.ShardSearchRequest;
import org.elasticsearch.threadpool.ThreadPool;

/**
* If field level security is enabled this interceptor disables the request cache for search requests.
* If field level security is enabled this interceptor disables the request cache for search and shardSearch requests.
*/
public class SearchRequestInterceptor extends FieldAndDocumentLevelSecurityRequestInterceptor {

Expand All @@ -26,14 +28,25 @@ public SearchRequestInterceptor(ThreadPool threadPool, XPackLicenseState license
@Override
public void disableFeatures(IndicesRequest indicesRequest, boolean fieldLevelSecurityEnabled, boolean documentLevelSecurityEnabled,
ActionListener<Void> listener) {
final SearchRequest request = (SearchRequest) indicesRequest;
request.requestCache(false);
assert indicesRequest instanceof SearchRequest || indicesRequest instanceof ShardSearchRequest
: "request must be either SearchRequest or ShardSearchRequest";

final SearchSourceBuilder source;
if (indicesRequest instanceof SearchRequest) {
final SearchRequest request = (SearchRequest) indicesRequest;
request.requestCache(false);
source = request.source();
} else {
final ShardSearchRequest request = (ShardSearchRequest) indicesRequest;
request.requestCache(false);
source = request.source();
}

if (documentLevelSecurityEnabled) {
if (request.source() != null && request.source().suggest() != null) {
if (source != null && source.suggest() != null) {
listener.onFailure(new ElasticsearchSecurityException("Suggest isn't supported if document level security is enabled",
RestStatus.BAD_REQUEST));
} else if (request.source() != null && request.source().profile()) {
} else if (source != null && source.profile()) {
listener.onFailure(new ElasticsearchSecurityException("A search request cannot be profiled if document level security " +
"is enabled", RestStatus.BAD_REQUEST));
} else {
Expand All @@ -46,6 +59,6 @@ public void disableFeatures(IndicesRequest indicesRequest, boolean fieldLevelSec

@Override
public boolean supports(IndicesRequest request) {
return request instanceof SearchRequest;
return request instanceof SearchRequest || request instanceof ShardSearchRequest;
}
}
65 changes: 43 additions & 22 deletions x-pack/qa/multi-cluster-search-security/build.gradle
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import org.elasticsearch.gradle.info.BuildParams
import org.elasticsearch.gradle.test.RestIntegTestTask

apply plugin: 'elasticsearch.testclusters'
Expand All @@ -19,36 +20,56 @@ task 'remote-cluster'(type: RestIntegTestTask) {
systemProperty 'tests.rest.suite', 'remote_cluster'
}

testClusters.'remote-cluster' {
testDistribution = 'DEFAULT'
numberOfNodes = 2
setting 'node.roles', '[data,ingest,master]'
setting 'xpack.security.enabled', 'true'
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.ml.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'
// randomise between sniff and proxy modes
boolean proxyMode = (new Random(Long.parseUnsignedLong(BuildParams.testSeed.tokenize(':').get(0), 16))).nextBoolean()

user username: "test_user", password: "x-pack-test-password"
testClusters {
'remote-cluster' {
testDistribution = 'DEFAULT'
numberOfNodes = 2
setting 'node.roles', '[data,ingest,master]'
setting 'xpack.security.enabled', 'true'
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.ml.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'

user username: "test_user", password: "x-pack-test-password"
}

'mixed-cluster' {
testDistribution = 'DEFAULT'
setting 'xpack.security.enabled', 'true'
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.ml.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'
if (proxyMode) {
setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
setting 'cluster.remote.my_remote_cluster.proxy_address', {
"\"${testClusters.'remote-cluster'.getAllTransportPortURI().get(0)}\""
}
} else {
setting 'cluster.remote.my_remote_cluster.seeds', {
testClusters.'remote-cluster'.getAllTransportPortURI().collect { "\"$it\"" }.toString()
}
}
setting 'cluster.remote.connections_per_cluster', "1"

user username: "test_user", password: "x-pack-test-password"
}
}

task 'mixed-cluster'(type: RestIntegTestTask) {
dependsOn 'remote-cluster'
useCluster testClusters.'remote-cluster'
systemProperty 'tests.rest.suite', 'multi_cluster'
}

testClusters.'mixed-cluster' {
testDistribution = 'DEFAULT'
setting 'xpack.security.enabled', 'true'
setting 'xpack.watcher.enabled', 'false'
setting 'xpack.ml.enabled', 'false'
setting 'xpack.license.self_generated.type', 'trial'
setting 'cluster.remote.my_remote_cluster.seeds', {
testClusters.'remote-cluster'.getAllTransportPortURI().collect { "\"$it\"" }.toString()
if (proxyMode) {
systemProperty 'tests.rest.blacklist', [
'multi_cluster/10_basic/Add transient remote cluster based on the preset cluster',
'multi_cluster/20_info/Add transient remote cluster based on the preset cluster and check remote info',
'multi_cluster/20_info/Fetch remote cluster info for existing cluster',
'multi_cluster/70_connection_mode_configuration/*',
].join(",")
}
setting 'cluster.remote.connections_per_cluster', "1"

user username: "test_user", password: "x-pack-test-password"
}

tasks.register("integTest") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@
- match: {indices.7.attributes.0: open }
- match: {indices.8.name: my_remote_cluster:secured_via_alias}
- match: {indices.8.attributes.0: open}
- match: {indices.9.name: my_remote_cluster:single_doc_index}
- match: {indices.10.attributes.0: open}
- match: {indices.10.name: my_remote_cluster:test_index}
- match: {indices.10.aliases.0: aliased_test_index}
- match: {indices.10.attributes.0: open}
- match: {indices.9.name: my_remote_cluster:shared_index}
- match: {indices.10.name: my_remote_cluster:single_doc_index}
- match: {indices.11.attributes.0: open}
- match: {indices.11.name: my_remote_cluster:test_index}
- match: {indices.11.aliases.0: aliased_test_index}
- match: {indices.11.attributes.0: open}
- match: {aliases.0.name: my_remote_cluster:.security}
- match: {aliases.0.indices.0: .security-7}
- match: {aliases.1.name: my_remote_cluster:aliased_closed_index}
Expand Down
Loading