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

Use reader wrapper in ReadOnlyEngine #75724

Merged
merged 2 commits into from
Jul 27, 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 @@ -566,7 +566,7 @@ public ShardLongFieldRange getRawFieldRange(String field) throws IOException {
@Override
public SearcherSupplier acquireSearcherSupplier(Function<Searcher, Searcher> wrapper, SearcherScope scope) throws EngineException {
final SearcherSupplier delegate = super.acquireSearcherSupplier(wrapper, scope);
return new SearcherSupplier(Function.identity()) {
return new SearcherSupplier(wrapper) {
@Override
protected void doClose() {
delegate.close();
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugin/searchable-snapshots/qa/rest/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ final File repoDir = file("$buildDir/testclusters/repo")

restResources {
restApi {
include 'indices', 'search', 'bulk', 'snapshot', 'nodes', '_common', 'searchable_snapshots', 'cluster', 'open_point_in_time', 'close_point_in_time'
include 'indices', 'search', 'bulk', 'snapshot', 'nodes', '_common', 'searchable_snapshots', 'cluster', 'open_point_in_time', 'close_point_in_time', 'security'
}
}

Expand All @@ -26,5 +26,6 @@ testClusters.all {
setting 'xpack.searchable.snapshot.shared_cache.size', '16MB'
setting 'xpack.searchable.snapshot.shared_cache.region_size', '256KB'

setting 'xpack.security.enabled', 'false'
setting 'xpack.security.enabled', 'true'
user username: 'admin', password: 'admin-password', role: 'superuser'
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
*/
package org.elasticsearch.xpack.searchablesnapshots.rest;

import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.repositories.fs.FsRepository;
import org.elasticsearch.xpack.searchablesnapshots.AbstractSearchableSnapshotsRestTestCase;

Expand All @@ -21,4 +23,12 @@ protected String writeRepositoryType() {
protected Settings writeRepositorySettings() {
return Settings.builder().put("location", System.getProperty("tests.path.repo")).build();
}

@Override
protected Settings restClientSettings() {
String token = basicAuthHeaderValue("admin", new SecureString("admin-password".toCharArray()));
return Settings.builder()
.put(ThreadContext.PREFIX + ".Authorization", token)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase;

Expand All @@ -21,4 +24,12 @@ public SearchableSnapshotsClientYamlTestSuiteIT(final ClientYamlTestCandidate te
public static Iterable<Object[]> parameters() throws Exception {
return ESClientYamlSuiteTestCase.createParameters();
}

@Override
protected Settings restClientSettings() {
String token = basicAuthHeaderValue("admin", new SecureString("admin-password".toCharArray()));
return Settings.builder()
.put(ThreadContext.PREFIX + ".Authorization", token)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,249 @@
---
setup:
- skip:
features: headers

- do:
cluster.health:
wait_for_status: yellow

- do:
security.put_role:
name: "readall"
body: >
{
"indices": [
{
"names": ["*"],
"privileges": ["read"]
}
]
}

- do:
security.put_role:
name: "limitread"
body: >
{
"indices": [
{
"names": ["*"],
"privileges": ["read"],
"query": {"match": {"marker": "test_1"}}
}
]
}

- do:
security.put_user:
username: "full"
body: >
{
"password" : "x-pack-test-password",
"roles" : [ "readall" ],
"full_name" : "user who can read all data"
}

- do:
security.put_user:
username: "limited"
body: >
{
"password" : "x-pack-test-password",
"roles" : [ "limitread" ],
"full_name" : "user who can read some data"
}

- do:
snapshot.create_repository:
repository: repository-fs
body:
type: fs
settings:
location: "repository-fs"

# Remove the snapshot if a previous test failed to delete it.
# Useful for third party tests that runs the test against a real external service.
- do:
snapshot.delete:
repository: repository-fs
snapshot: snapshot
ignore: 404

- do:
indices.create:
index: test_index
body:
mappings:
properties:
location:
properties:
city:
type: "keyword"
settings:
index:
number_of_shards: 1
number_of_replicas: 0

- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "test_index"}}'
- '{"marker": "test_1", "location.city": "bos"}'
- '{"index": {"_index": "test_index"}}'
- '{"marker": "test_2", "location.city": "ams"}'

- do:
snapshot.create:
repository: repository-fs
snapshot: snapshot
wait_for_completion: true

- do:
indices.delete:
index: test_index

---
teardown:
- do:
security.delete_user:
username: "full"
ignore: 404

- do:
security.delete_user:
username: "limited"
ignore: 404

- do:
security.delete_role:
name: "readall"
ignore: 404

- do:
security.delete_role:
name: "limitread"
ignore: 404

---
"Test doc level security with different users on full_copy index":

- do:
searchable_snapshots.mount:
repository: repository-fs
snapshot: snapshot
wait_for_completion: true
storage: full_copy
body:
index: test_index
renamed_index: test_index

- match: { snapshot.snapshot: snapshot }
- match: { snapshot.shards.failed: 0 }
- match: { snapshot.shards.successful: 1 }

- do:
headers: { Authorization: "Basic ZnVsbDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # full - user
search:
rest_total_hits_as_int: true
index: test_index
size: 0
from: 0
body:
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { hits.total: 2 }
- length: { aggregations.cities.buckets: 2 }
- match: { aggregations.cities.buckets.0.key: "ams" }
- match: { aggregations.cities.buckets.0.doc_count: 1 }
- match: { aggregations.cities.buckets.1.key: "bos" }
- match: { aggregations.cities.buckets.1.doc_count: 1 }

- do:
headers: { Authorization: "Basic bGltaXRlZDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # limited - user
search:
rest_total_hits_as_int: true
index: test_index
size: 0
from: 0
body:
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { hits.total: 1 }
- length: { aggregations.cities.buckets: 1 }
- match: { aggregations.cities.buckets.0.key: "bos" }
- match: { aggregations.cities.buckets.0.doc_count: 1 }

- do:
indices.delete:
index: test_index

---
"Test doc level security with different users on shared_cache index":

- do:
searchable_snapshots.mount:
repository: repository-fs
snapshot: snapshot
wait_for_completion: true
storage: shared_cache
body:
index: test_index
renamed_index: test_index

- match: { snapshot.snapshot: snapshot }
- match: { snapshot.shards.failed: 0 }
- match: { snapshot.shards.successful: 1 }

- do:
headers: { Authorization: "Basic ZnVsbDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # full - user
search:
rest_total_hits_as_int: true
index: test_index
size: 0
from: 0
body:
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { hits.total: 2 }
- length: { aggregations.cities.buckets: 2 }
- match: { aggregations.cities.buckets.0.key: "ams" }
- match: { aggregations.cities.buckets.0.doc_count: 1 }
- match: { aggregations.cities.buckets.1.key: "bos" }
- match: { aggregations.cities.buckets.1.doc_count: 1 }

- do:
headers: { Authorization: "Basic bGltaXRlZDp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # limited - user
search:
rest_total_hits_as_int: true
index: test_index
size: 0
from: 0
body:
aggs:
cities:
terms:
field: location.city

- match: { _shards.total: 1 }
- match: { hits.total: 1 }
- length: { aggregations.cities.buckets: 1 }
- match: { aggregations.cities.buckets.0.key: "bos" }
- match: { aggregations.cities.buckets.0.doc_count: 1 }

- do:
indices.delete:
index: test_index
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ setup:

- is_true: nodes.$node_id
- is_true: nodes.$node_id.shared_cache
- match: { nodes.$node_id.shared_cache.reads: 0 }
- match: { nodes.$node_id.shared_cache.bytes_read: "0b" }
- match: { nodes.$node_id.shared_cache.bytes_read_in_bytes: 0 }
- match: { nodes.$node_id.shared_cache.writes: 0 }
- match: { nodes.$node_id.shared_cache.bytes_written: "0b" }
- match: { nodes.$node_id.shared_cache.bytes_written_in_bytes: 0 }
- match: { nodes.$node_id.shared_cache.evictions: 0 }
- gte: { nodes.$node_id.shared_cache.reads: 0 }
Copy link
Contributor Author

@ywelsch ywelsch Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting side-effect here because we now actually start using the cache.

- gte: { nodes.$node_id.shared_cache.bytes_read: "0b" }
- gte: { nodes.$node_id.shared_cache.bytes_read_in_bytes: 0 }
- gte: { nodes.$node_id.shared_cache.writes: 0 }
- gte: { nodes.$node_id.shared_cache.bytes_written: "0b" }
- gte: { nodes.$node_id.shared_cache.bytes_written_in_bytes: 0 }
- gte: { nodes.$node_id.shared_cache.evictions: 0 }
- match: { nodes.$node_id.shared_cache.num_regions: 64 }
- match: { nodes.$node_id.shared_cache.size: "16mb" }
- match: { nodes.$node_id.shared_cache.size_in_bytes: 16777216 }
Expand Down