From 3cfb649661438f002816b1c9bbd17d78c14827a6 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:29:57 +1100 Subject: [PATCH 1/5] Mute org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT test {p0=search.highlight/50_synthetic_source/text multi unified from vectors} #117815 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index b82e95ea26890..8d64e1557ca19 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -234,6 +234,9 @@ tests: - class: org.elasticsearch.search.ccs.CrossClusterIT method: testCancel issue: https://github.com/elastic/elasticsearch/issues/108061 +- class: org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT + method: test {p0=search.highlight/50_synthetic_source/text multi unified from vectors} + issue: https://github.com/elastic/elasticsearch/issues/117815 # Examples: # From 2b7adcd89dfb31411f68dda211f689d42b979af8 Mon Sep 17 00:00:00 2001 From: Kostas Krikellas <131142368+kkrik-es@users.noreply.github.com> Date: Mon, 2 Dec 2024 09:58:25 +0200 Subject: [PATCH 2/5] Add debug logging for doc parsing exceptions (#117768) --- .../index/mapper/DocumentMapper.java | 23 ++++++++++++++++--- .../index/mapper/MapperService.java | 9 +++++++- .../index/mapper/DocumentMapperTests.java | 3 ++- .../index/mapper/DocumentParserTests.java | 3 ++- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 10484a1c26098..1c9321737ab5f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -9,7 +9,9 @@ package org.elasticsearch.index.mapper; +import org.apache.logging.log4j.Logger; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.features.NodeFeature; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexSortConfig; @@ -25,6 +27,7 @@ public class DocumentMapper { private final DocumentParser documentParser; private final MapperMetrics mapperMetrics; private final IndexVersion indexVersion; + private final Logger logger; static final NodeFeature INDEX_SORTING_ON_NESTED = new NodeFeature("mapper.index_sorting_on_nested"); @@ -44,7 +47,8 @@ public static DocumentMapper createEmpty(MapperService mapperService) { mapping, mapping.toCompressedXContent(), IndexVersion.current(), - mapperService.getMapperMetrics() + mapperService.getMapperMetrics(), + mapperService.index().getName() ); } @@ -53,7 +57,8 @@ public static DocumentMapper createEmpty(MapperService mapperService) { Mapping mapping, CompressedXContent source, IndexVersion version, - MapperMetrics mapperMetrics + MapperMetrics mapperMetrics, + String indexName ) { this.documentParser = documentParser; this.type = mapping.getRoot().fullPath(); @@ -61,11 +66,18 @@ public static DocumentMapper createEmpty(MapperService mapperService) { this.mappingSource = source; this.mapperMetrics = mapperMetrics; this.indexVersion = version; + this.logger = Loggers.getLogger(getClass(), indexName); assert mapping.toCompressedXContent().equals(source) || isSyntheticSourceMalformed(source, version) : "provided source [" + source + "] differs from mapping [" + mapping.toCompressedXContent() + "]"; } + private void maybeLogDebug(Exception ex) { + if (logger.isDebugEnabled()) { + logger.debug("Error while parsing document: " + ex.getMessage(), ex); + } + } + /** * Indexes built at v.8.7 were missing an explicit entry for synthetic_source. * This got restored in v.8.10 to avoid confusion. The change is only restricted to mapping printout, it has no @@ -110,7 +122,12 @@ public MappingLookup mappers() { } public ParsedDocument parse(SourceToParse source) throws DocumentParsingException { - return documentParser.parseDocument(source, mappingLookup); + try { + return documentParser.parseDocument(source, mappingLookup); + } catch (Exception e) { + maybeLogDebug(e); + throw e; + } } public void validate(IndexSettings settings, boolean checkLimits) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 7f952153c6453..1673b1719d8bf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -585,7 +585,14 @@ private DocumentMapper doMerge(String type, MergeReason reason, Map Date: Mon, 2 Dec 2024 08:11:09 +0000 Subject: [PATCH 3/5] Revert "(+Doc) Link split-brain wiki (#108914)" This reverts commit 12aab083301958ddfbeec9ee09d333da8278fd2c. --- docs/reference/modules/discovery/voting.asciidoc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/reference/modules/discovery/voting.asciidoc b/docs/reference/modules/discovery/voting.asciidoc index 9e483d5883017..04cae9d02ab66 100644 --- a/docs/reference/modules/discovery/voting.asciidoc +++ b/docs/reference/modules/discovery/voting.asciidoc @@ -63,8 +63,7 @@ departed nodes from the voting configuration manually. Use the of resilience. No matter how it is configured, Elasticsearch will not suffer from a -"{wikipedia}/Split-brain_(computing)[split-brain]" inconsistency. -The `cluster.auto_shrink_voting_configuration` +"split-brain" inconsistency. The `cluster.auto_shrink_voting_configuration` setting affects only its availability in the event of the failure of some of its nodes and the administrative tasks that must be performed as nodes join and leave the cluster. From 9dcd9751f481952f5f08332b15aed31179af324d Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 2 Dec 2024 09:01:48 +0000 Subject: [PATCH 4/5] Add IMDSv2 support to `repository-s3` (#117748) The version of the AWS Java SDK we use already magically switches to IMDSv2 if available, but today we cannot claim to support IMDSv2 in Elasticsearch since we have no tests demonstrating that the magic really works for us. In particular, this sort of thing often risks falling foul of some restrictions imposed by the security manager (if not now then maybe in some future release). This commit adds proper support for IMDSv2 by enhancing the test suite to add the missing coverage to avoid any risk of breaking this magical SDK behaviour in future. Closes #105135 Closes ES-9984 --- docs/changelog/117748.yaml | 6 ++ .../snapshot-restore/repository-s3.asciidoc | 42 ++++++----- .../s3/RepositoryS3EcsCredentialsRestIT.java | 2 + .../RepositoryS3ImdsV1CredentialsRestIT.java | 2 + .../RepositoryS3ImdsV2CredentialsRestIT.java | 75 +++++++++++++++++++ .../fixture/aws/imds/Ec2ImdsHttpFixture.java | 10 ++- .../fixture/aws/imds/Ec2ImdsHttpHandler.java | 35 ++++++++- .../java/fixture/aws/imds/Ec2ImdsVersion.java | 26 +++++++ .../aws/imds/Ec2ImdsHttpHandlerTests.java | 67 +++++++++++++++-- 9 files changed, 236 insertions(+), 29 deletions(-) create mode 100644 docs/changelog/117748.yaml create mode 100644 modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java create mode 100644 test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsVersion.java diff --git a/docs/changelog/117748.yaml b/docs/changelog/117748.yaml new file mode 100644 index 0000000000000..615adbae07ad7 --- /dev/null +++ b/docs/changelog/117748.yaml @@ -0,0 +1,6 @@ +pr: 117748 +summary: Add IMDSv2 support to `repository-s3` +area: Snapshot/Restore +type: enhancement +issues: + - 105135 diff --git a/docs/reference/snapshot-restore/repository-s3.asciidoc b/docs/reference/snapshot-restore/repository-s3.asciidoc index 1b08a802a444f..9b71fe9220385 100644 --- a/docs/reference/snapshot-restore/repository-s3.asciidoc +++ b/docs/reference/snapshot-restore/repository-s3.asciidoc @@ -38,7 +38,8 @@ PUT _snapshot/my_s3_repository The client that you use to connect to S3 has a number of settings available. The settings have the form `s3.client.CLIENT_NAME.SETTING_NAME`. By default, `s3` repositories use a client named `default`, but this can be modified using -the <> `client`. For example: +the <> `client`. For example, to +use a client named `my-alternate-client`, register the repository as follows: [source,console] ---- @@ -69,10 +70,19 @@ bin/elasticsearch-keystore add s3.client.default.secret_key bin/elasticsearch-keystore add s3.client.default.session_token ---- -If instead you want to use the instance role or container role to access S3 -then you should leave these settings unset. You can switch from using specific -credentials back to the default of using the instance role or container role by -removing these settings from the keystore as follows: +If you do not configure these settings then {es} will attempt to automatically +obtain credentials from the environment in which it is running: + +* Nodes running on an instance in AWS EC2 will attempt to use the EC2 Instance + Metadata Service (IMDS) to obtain instance role credentials. {es} supports + both IMDS version 1 and IMDS version 2. + +* Nodes running in a container in AWS ECS and AWS EKS will attempt to obtain + container role credentials similarly. + +You can switch from using specific credentials back to the default of using the +instance role or container role by removing these settings from the keystore as +follows: [source,sh] ---- @@ -82,20 +92,14 @@ bin/elasticsearch-keystore remove s3.client.default.secret_key bin/elasticsearch-keystore remove s3.client.default.session_token ---- -*All* client secure settings of this repository type are -{ref}/secure-settings.html#reloadable-secure-settings[reloadable]. -You can define these settings before the node is started, -or call the <> -after the settings are defined to apply them to a running node. - -After you reload the settings, the internal `s3` clients, used to transfer the snapshot -contents, will utilize the latest settings from the keystore. Any existing `s3` -repositories, as well as any newly created ones, will pick up the new values -stored in the keystore. - -NOTE: In-progress snapshot/restore tasks will not be preempted by a *reload* of -the client's secure settings. The task will complete using the client as it was -built when the operation started. +Define the relevant secure settings in each node's keystore before starting the +node. The secure settings described here are all +{ref}/secure-settings.html#reloadable-secure-settings[reloadable] so you may +update the keystore contents on each node while the node is running and then +call the <> to apply the updated settings to the nodes in the cluster. After this API +completes, {es} will use the updated setting values for all future snapshot +operations, but ongoing operations may continue to use older setting values. The following list contains the available client settings. Those that must be stored in the keystore are marked as "secure" and are *reloadable*; the other diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java index 267ba6e6b3a13..a79ae4de7cc66 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3EcsCredentialsRestIT.java @@ -10,6 +10,7 @@ package org.elasticsearch.repositories.s3; import fixture.aws.imds.Ec2ImdsHttpFixture; +import fixture.aws.imds.Ec2ImdsVersion; import fixture.s3.DynamicS3Credentials; import fixture.s3.S3HttpFixture; @@ -36,6 +37,7 @@ public class RepositoryS3EcsCredentialsRestIT extends AbstractRepositoryS3RestTe private static final DynamicS3Credentials dynamicS3Credentials = new DynamicS3Credentials(); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( + Ec2ImdsVersion.V1, dynamicS3Credentials::addValidCredentials, Set.of("/ecs_credentials_endpoint") ); diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java index de9c9b6ae0695..ead91981b3fa8 100644 --- a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV1CredentialsRestIT.java @@ -10,6 +10,7 @@ package org.elasticsearch.repositories.s3; import fixture.aws.imds.Ec2ImdsHttpFixture; +import fixture.aws.imds.Ec2ImdsVersion; import fixture.s3.DynamicS3Credentials; import fixture.s3.S3HttpFixture; @@ -36,6 +37,7 @@ public class RepositoryS3ImdsV1CredentialsRestIT extends AbstractRepositoryS3Res private static final DynamicS3Credentials dynamicS3Credentials = new DynamicS3Credentials(); private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( + Ec2ImdsVersion.V1, dynamicS3Credentials::addValidCredentials, Set.of() ); diff --git a/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java new file mode 100644 index 0000000000000..67adb096bd1ba --- /dev/null +++ b/modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ImdsV2CredentialsRestIT.java @@ -0,0 +1,75 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.repositories.s3; + +import fixture.aws.imds.Ec2ImdsHttpFixture; +import fixture.aws.imds.Ec2ImdsVersion; +import fixture.s3.DynamicS3Credentials; +import fixture.s3.S3HttpFixture; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; + +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; + +import java.util.Set; + +@ThreadLeakFilters(filters = { TestContainersThreadFilter.class }) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) // https://github.com/elastic/elasticsearch/issues/102482 +public class RepositoryS3ImdsV2CredentialsRestIT extends AbstractRepositoryS3RestTestCase { + + private static final String PREFIX = getIdentifierPrefix("RepositoryS3ImdsV2CredentialsRestIT"); + private static final String BUCKET = PREFIX + "bucket"; + private static final String BASE_PATH = PREFIX + "base_path"; + private static final String CLIENT = "imdsv2_credentials_client"; + + private static final DynamicS3Credentials dynamicS3Credentials = new DynamicS3Credentials(); + + private static final Ec2ImdsHttpFixture ec2ImdsHttpFixture = new Ec2ImdsHttpFixture( + Ec2ImdsVersion.V2, + dynamicS3Credentials::addValidCredentials, + Set.of() + ); + + private static final S3HttpFixture s3Fixture = new S3HttpFixture(true, BUCKET, BASE_PATH, dynamicS3Credentials::isAuthorized); + + public static ElasticsearchCluster cluster = ElasticsearchCluster.local() + .module("repository-s3") + .setting("s3.client." + CLIENT + ".endpoint", s3Fixture::getAddress) + .systemProperty("com.amazonaws.sdk.ec2MetadataServiceEndpointOverride", ec2ImdsHttpFixture::getAddress) + .build(); + + @ClassRule + public static TestRule ruleChain = RuleChain.outerRule(ec2ImdsHttpFixture).around(s3Fixture).around(cluster); + + @Override + protected String getTestRestCluster() { + return cluster.getHttpAddresses(); + } + + @Override + protected String getBucketName() { + return BUCKET; + } + + @Override + protected String getBasePath() { + return BASE_PATH; + } + + @Override + protected String getClientName() { + return CLIENT; + } +} diff --git a/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpFixture.java b/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpFixture.java index 13d36c6fc4812..c63c65a750d7c 100644 --- a/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpFixture.java +++ b/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpFixture.java @@ -24,16 +24,22 @@ public class Ec2ImdsHttpFixture extends ExternalResource { private HttpServer server; + private final Ec2ImdsVersion ec2ImdsVersion; private final BiConsumer newCredentialsConsumer; private final Set alternativeCredentialsEndpoints; - public Ec2ImdsHttpFixture(BiConsumer newCredentialsConsumer, Set alternativeCredentialsEndpoints) { + public Ec2ImdsHttpFixture( + Ec2ImdsVersion ec2ImdsVersion, + BiConsumer newCredentialsConsumer, + Set alternativeCredentialsEndpoints + ) { + this.ec2ImdsVersion = Objects.requireNonNull(ec2ImdsVersion); this.newCredentialsConsumer = Objects.requireNonNull(newCredentialsConsumer); this.alternativeCredentialsEndpoints = Objects.requireNonNull(alternativeCredentialsEndpoints); } protected HttpHandler createHandler() { - return new Ec2ImdsHttpHandler(newCredentialsConsumer, alternativeCredentialsEndpoints); + return new Ec2ImdsHttpHandler(ec2ImdsVersion, newCredentialsConsumer, alternativeCredentialsEndpoints); } public String getAddress() { diff --git a/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpHandler.java b/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpHandler.java index bc87eff592bec..281465b96de05 100644 --- a/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpHandler.java +++ b/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsHttpHandler.java @@ -38,10 +38,18 @@ public class Ec2ImdsHttpHandler implements HttpHandler { private static final String IMDS_SECURITY_CREDENTIALS_PATH = "/latest/meta-data/iam/security-credentials/"; + private final Ec2ImdsVersion ec2ImdsVersion; + private final Set validImdsTokens = ConcurrentCollections.newConcurrentSet(); + private final BiConsumer newCredentialsConsumer; private final Set validCredentialsEndpoints = ConcurrentCollections.newConcurrentSet(); - public Ec2ImdsHttpHandler(BiConsumer newCredentialsConsumer, Collection alternativeCredentialsEndpoints) { + public Ec2ImdsHttpHandler( + Ec2ImdsVersion ec2ImdsVersion, + BiConsumer newCredentialsConsumer, + Collection alternativeCredentialsEndpoints + ) { + this.ec2ImdsVersion = Objects.requireNonNull(ec2ImdsVersion); this.newCredentialsConsumer = Objects.requireNonNull(newCredentialsConsumer); this.validCredentialsEndpoints.addAll(alternativeCredentialsEndpoints); } @@ -55,11 +63,32 @@ public void handle(final HttpExchange exchange) throws IOException { final var requestMethod = exchange.getRequestMethod(); if ("PUT".equals(requestMethod) && "/latest/api/token".equals(path)) { - // Reject IMDSv2 probe - exchange.sendResponseHeaders(RestStatus.METHOD_NOT_ALLOWED.getStatus(), -1); + switch (ec2ImdsVersion) { + case V1 -> exchange.sendResponseHeaders(RestStatus.METHOD_NOT_ALLOWED.getStatus(), -1); + case V2 -> { + final var token = randomSecretKey(); + validImdsTokens.add(token); + final var responseBody = token.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "text/plain"); + exchange.sendResponseHeaders(RestStatus.OK.getStatus(), responseBody.length); + exchange.getResponseBody().write(responseBody); + } + } return; } + if (ec2ImdsVersion == Ec2ImdsVersion.V2) { + final var token = exchange.getRequestHeaders().getFirst("X-aws-ec2-metadata-token"); + if (token == null) { + exchange.sendResponseHeaders(RestStatus.UNAUTHORIZED.getStatus(), -1); + return; + } + if (validImdsTokens.contains(token) == false) { + exchange.sendResponseHeaders(RestStatus.FORBIDDEN.getStatus(), -1); + return; + } + } + if ("GET".equals(requestMethod)) { if (path.equals(IMDS_SECURITY_CREDENTIALS_PATH)) { final var profileName = randomIdentifier(); diff --git a/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsVersion.java b/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsVersion.java new file mode 100644 index 0000000000000..7ed028c374cc7 --- /dev/null +++ b/test/fixtures/ec2-imds-fixture/src/main/java/fixture/aws/imds/Ec2ImdsVersion.java @@ -0,0 +1,26 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package fixture.aws.imds; + +/** + * Represents the IMDS protocol version simulated by the {@link Ec2ImdsHttpHandler}. + */ +public enum Ec2ImdsVersion { + /** + * Classic V1 behavior: plain {@code GET} requests, no tokens. + */ + V1, + + /** + * Newer V2 behavior: {@code GET} requests must include a {@code X-aws-ec2-metadata-token} header providing a token previously obtained + * by calling {@code PUT /latest/api/token}. + */ + V2 +} diff --git a/test/fixtures/ec2-imds-fixture/src/test/java/fixture/aws/imds/Ec2ImdsHttpHandlerTests.java b/test/fixtures/ec2-imds-fixture/src/test/java/fixture/aws/imds/Ec2ImdsHttpHandlerTests.java index 369b0ef449b2f..bb613395a0fba 100644 --- a/test/fixtures/ec2-imds-fixture/src/test/java/fixture/aws/imds/Ec2ImdsHttpHandlerTests.java +++ b/test/fixtures/ec2-imds-fixture/src/test/java/fixture/aws/imds/Ec2ImdsHttpHandlerTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.Nullable; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentType; @@ -29,6 +30,7 @@ import java.net.InetSocketAddress; import java.net.URI; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; @@ -36,17 +38,19 @@ public class Ec2ImdsHttpHandlerTests extends ESTestCase { + private static final String SECURITY_CREDENTIALS_URI = "/latest/meta-data/iam/security-credentials/"; + public void testImdsV1() throws IOException { final Map generatedCredentials = new HashMap<>(); - final var handler = new Ec2ImdsHttpHandler(generatedCredentials::put, Set.of()); + final var handler = new Ec2ImdsHttpHandler(Ec2ImdsVersion.V1, generatedCredentials::put, Set.of()); - final var roleResponse = handleRequest(handler, "GET", "/latest/meta-data/iam/security-credentials/"); + final var roleResponse = handleRequest(handler, "GET", SECURITY_CREDENTIALS_URI); assertEquals(RestStatus.OK, roleResponse.status()); final var profileName = roleResponse.body().utf8ToString(); assertTrue(Strings.hasText(profileName)); - final var credentialsResponse = handleRequest(handler, "GET", "/latest/meta-data/iam/security-credentials/" + profileName); + final var credentialsResponse = handleRequest(handler, "GET", SECURITY_CREDENTIALS_URI + profileName); assertEquals(RestStatus.OK, credentialsResponse.status()); assertThat(generatedCredentials, aMapWithSize(1)); @@ -62,14 +66,67 @@ public void testImdsV1() throws IOException { public void testImdsV2Disabled() { assertEquals( RestStatus.METHOD_NOT_ALLOWED, - handleRequest(new Ec2ImdsHttpHandler((accessKey, sessionToken) -> fail(), Set.of()), "PUT", "/latest/api/token").status() + handleRequest( + new Ec2ImdsHttpHandler(Ec2ImdsVersion.V1, (accessKey, sessionToken) -> fail(), Set.of()), + "PUT", + "/latest/api/token" + ).status() ); } + public void testImdsV2() throws IOException { + final Map generatedCredentials = new HashMap<>(); + + final var handler = new Ec2ImdsHttpHandler(Ec2ImdsVersion.V2, generatedCredentials::put, Set.of()); + + final var tokenResponse = handleRequest(handler, "PUT", "/latest/api/token"); + assertEquals(RestStatus.OK, tokenResponse.status()); + final var token = tokenResponse.body().utf8ToString(); + + final var roleResponse = checkImdsV2GetRequest(handler, SECURITY_CREDENTIALS_URI, token); + assertEquals(RestStatus.OK, roleResponse.status()); + final var profileName = roleResponse.body().utf8ToString(); + assertTrue(Strings.hasText(profileName)); + + final var credentialsResponse = checkImdsV2GetRequest(handler, SECURITY_CREDENTIALS_URI + profileName, token); + assertEquals(RestStatus.OK, credentialsResponse.status()); + + assertThat(generatedCredentials, aMapWithSize(1)); + final var accessKey = generatedCredentials.keySet().iterator().next(); + final var sessionToken = generatedCredentials.values().iterator().next(); + + final var responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), credentialsResponse.body().streamInput(), false); + assertEquals(Set.of("AccessKeyId", "Expiration", "RoleArn", "SecretAccessKey", "Token"), responseMap.keySet()); + assertEquals(accessKey, responseMap.get("AccessKeyId")); + assertEquals(sessionToken, responseMap.get("Token")); + } + private record TestHttpResponse(RestStatus status, BytesReference body) {} + private static TestHttpResponse checkImdsV2GetRequest(Ec2ImdsHttpHandler handler, String uri, String token) { + final var unauthorizedResponse = handleRequest(handler, "GET", uri, null); + assertEquals(RestStatus.UNAUTHORIZED, unauthorizedResponse.status()); + + final var forbiddenResponse = handleRequest(handler, "GET", uri, randomValueOtherThan(token, ESTestCase::randomSecretKey)); + assertEquals(RestStatus.FORBIDDEN, forbiddenResponse.status()); + + return handleRequest(handler, "GET", uri, token); + } + private static TestHttpResponse handleRequest(Ec2ImdsHttpHandler handler, String method, String uri) { - final var httpExchange = new TestHttpExchange(method, uri, BytesArray.EMPTY, TestHttpExchange.EMPTY_HEADERS); + return handleRequest(handler, method, uri, null); + } + + private static TestHttpResponse handleRequest(Ec2ImdsHttpHandler handler, String method, String uri, @Nullable String token) { + final Headers headers; + if (token == null) { + headers = TestHttpExchange.EMPTY_HEADERS; + } else { + headers = new Headers(); + headers.put("X-aws-ec2-metadata-token", List.of(token)); + } + + final var httpExchange = new TestHttpExchange(method, uri, BytesArray.EMPTY, headers); try { handler.handle(httpExchange); } catch (IOException e) { From d2a4c70ca1f85e408efdc572ed4dda847733b0be Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Mon, 2 Dec 2024 11:16:38 +0200 Subject: [PATCH 5/5] Search Queries in parallel - part 3 (#117149) Update IT tests grouping assertResponses --- .../elasticsearch/aliases/IndexAliasesIT.java | 74 ++----- .../fetch/subphase/MatchedQueriesIT.java | 109 ++++------ .../highlight/HighlighterSearchIT.java | 193 +++++++----------- .../search/functionscore/QueryRescorerIT.java | 74 +++---- .../search/query/MultiMatchQueryIT.java | 109 +++------- 5 files changed, 175 insertions(+), 384 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java index b70da34c8fe3f..309bf69f00be0 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java @@ -65,6 +65,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailuresAndResponse; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponses; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.emptyArray; @@ -262,27 +263,16 @@ public void testSearchingFilteringAliasesSingleIndex() throws Exception { .setRefreshPolicy(RefreshPolicy.IMMEDIATE) ).actionGet(); - logger.info("--> checking single filtering alias search"); - assertResponse( + assertResponses( + searchResponse -> assertHits(searchResponse.getHits(), "1"), prepareSearch("foos").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1") - ); - - logger.info("--> checking single filtering alias wildcard search"); - assertResponse( - prepareSearch("fo*").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1") + prepareSearch("fo*").setQuery(QueryBuilders.matchAllQuery()) ); - assertResponse( + assertResponses( + searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3"), prepareSearch("tests").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3") - ); - - logger.info("--> checking single filtering alias search with sort"); - assertResponse( - prepareSearch("tests").setQuery(QueryBuilders.matchAllQuery()).addSort("_index", SortOrder.ASC), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3") + prepareSearch("tests").setQuery(QueryBuilders.matchAllQuery()).addSort("_index", SortOrder.ASC) ); logger.info("--> checking single filtering alias search with global facets"); @@ -323,28 +313,12 @@ public void testSearchingFilteringAliasesSingleIndex() throws Exception { searchResponse -> assertHits(searchResponse.getHits(), "1", "2") ); - logger.info("--> checking single non-filtering alias search"); - assertResponse( + assertResponses( + searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3", "4"), prepareSearch("alias1").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3", "4") - ); - - logger.info("--> checking non-filtering alias and filtering alias search"); - assertResponse( prepareSearch("alias1", "foos").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3", "4") - ); - - logger.info("--> checking index and filtering alias search"); - assertResponse( prepareSearch("test", "foos").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3", "4") - ); - - logger.info("--> checking index and alias wildcard search"); - assertResponse( - prepareSearch("te*", "fo*").setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3", "4") + prepareSearch("te*", "fo*").setQuery(QueryBuilders.matchAllQuery()) ); } @@ -506,11 +480,11 @@ public void testSearchingFilteringAliasesMultipleIndices() throws Exception { prepareSearch("filter23", "filter13").setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertHits(searchResponse.getHits(), "21", "31", "13", "33") ); - assertResponse( + assertResponses( + searchResponse -> assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(4L)), prepareSearch("filter23", "filter13").setSize(0).setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(4L)) + prepareSearch("filter13", "filter1").setSize(0).setQuery(QueryBuilders.matchAllQuery()) ); - assertResponse( prepareSearch("filter23", "filter1").setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertHits(searchResponse.getHits(), "21", "31", "11", "12", "13") @@ -519,16 +493,10 @@ public void testSearchingFilteringAliasesMultipleIndices() throws Exception { prepareSearch("filter23", "filter1").setSize(0).setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(5L)) ); - assertResponse( prepareSearch("filter13", "filter1").setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertHits(searchResponse.getHits(), "11", "12", "13", "33") ); - assertResponse( - prepareSearch("filter13", "filter1").setSize(0).setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(4L)) - ); - assertResponse( prepareSearch("filter13", "filter1", "filter23").setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertHits(searchResponse.getHits(), "11", "12", "13", "21", "31", "33") @@ -537,7 +505,6 @@ public void testSearchingFilteringAliasesMultipleIndices() throws Exception { prepareSearch("filter13", "filter1", "filter23").setSize(0).setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(6L)) ); - assertResponse( prepareSearch("filter23", "filter13", "test2").setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertHits(searchResponse.getHits(), "21", "22", "23", "31", "13", "33") @@ -546,7 +513,6 @@ public void testSearchingFilteringAliasesMultipleIndices() throws Exception { prepareSearch("filter23", "filter13", "test2").setSize(0).setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(6L)) ); - assertResponse( prepareSearch("filter23", "filter13", "test1", "test2").setQuery(QueryBuilders.matchAllQuery()), searchResponse -> assertHits(searchResponse.getHits(), "11", "12", "13", "21", "22", "23", "31", "33") @@ -1325,17 +1291,13 @@ public void testIndexingAndQueryingHiddenAliases() throws Exception { searchResponse -> assertHits(searchResponse.getHits(), "2", "3") ); - // Ensure that all docs can be gotten through the alias - assertResponse( + assertResponses( + searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3"), + // Ensure that all docs can be gotten through the alias prepareSearch(alias).setQuery(QueryBuilders.matchAllQuery()), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3") - ); - - // And querying using a wildcard with indices options set to expand hidden - assertResponse( + // And querying using a wildcard with indices options set to expand hidden prepareSearch("alias*").setQuery(QueryBuilders.matchAllQuery()) - .setIndicesOptions(IndicesOptions.fromOptions(false, false, true, false, true, true, true, false, false)), - searchResponse -> assertHits(searchResponse.getHits(), "1", "2", "3") + .setIndicesOptions(IndicesOptions.fromOptions(false, false, true, false, true, true, true, false, false)) ); // And that querying the alias with a wildcard and no expand options fails diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/MatchedQueriesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/MatchedQueriesIT.java index c796522eda0e8..b0faeeb295e33 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/MatchedQueriesIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/MatchedQueriesIT.java @@ -33,6 +33,7 @@ import static org.elasticsearch.index.query.QueryBuilders.wrapperQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponses; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasKey; @@ -105,54 +106,32 @@ public void testSimpleMatchedQueryFromTopLevelFilter() throws Exception { prepareIndex("test").setId("3").setSource("name", "test").get(); refresh(); - assertResponse( + assertResponses(response -> { + assertHitCount(response, 3L); + for (SearchHit hit : response.getHits()) { + if (hit.getId().equals("1")) { + assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(2)); + assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); + assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); + assertThat(hit.getMatchedQueriesAndScores(), hasKey("title")); + assertThat(hit.getMatchedQueryScore("title"), greaterThan(0f)); + } else if (hit.getId().equals("2") || hit.getId().equals("3")) { + assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(1)); + assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); + assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); + } else { + fail("Unexpected document returned with id " + hit.getId()); + } + } + }, prepareSearch().setQuery(matchAllQuery()) .setPostFilter( boolQuery().should(termQuery("name", "test").queryName("name")).should(termQuery("title", "title1").queryName("title")) ), - response -> { - assertHitCount(response, 3L); - for (SearchHit hit : response.getHits()) { - if (hit.getId().equals("1")) { - assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(2)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); - assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("title")); - assertThat(hit.getMatchedQueryScore("title"), greaterThan(0f)); - } else if (hit.getId().equals("2") || hit.getId().equals("3")) { - assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(1)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); - assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); - } else { - fail("Unexpected document returned with id " + hit.getId()); - } - } - } - ); - - assertResponse( prepareSearch().setQuery(matchAllQuery()) .setPostFilter( boolQuery().should(termQuery("name", "test").queryName("name")).should(termQuery("title", "title1").queryName("title")) - ), - response -> { - assertHitCount(response, 3L); - for (SearchHit hit : response.getHits()) { - if (hit.getId().equals("1")) { - assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(2)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); - assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("title")); - assertThat(hit.getMatchedQueryScore("title"), greaterThan(0f)); - } else if (hit.getId().equals("2") || hit.getId().equals("3")) { - assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(1)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); - assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); - } else { - fail("Unexpected document returned with id " + hit.getId()); - } - } - } + ) ); } @@ -165,43 +144,25 @@ public void testSimpleMatchedQueryFromTopLevelFilterAndFilteredQuery() throws Ex prepareIndex("test").setId("3").setSource("name", "test", "title", "title3").get(); refresh(); - assertResponse( + assertResponses(response -> { + assertHitCount(response, 3L); + for (SearchHit hit : response.getHits()) { + if (hit.getId().equals("1") || hit.getId().equals("2") || hit.getId().equals("3")) { + assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(2)); + assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); + assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); + assertThat(hit.getMatchedQueriesAndScores(), hasKey("title")); + assertThat(hit.getMatchedQueryScore("title"), greaterThan(0f)); + } else { + fail("Unexpected document returned with id " + hit.getId()); + } + } + }, prepareSearch().setQuery( boolQuery().must(matchAllQuery()).filter(termsQuery("title", "title1", "title2", "title3").queryName("title")) ).setPostFilter(termQuery("name", "test").queryName("name")), - response -> { - assertHitCount(response, 3L); - for (SearchHit hit : response.getHits()) { - if (hit.getId().equals("1") || hit.getId().equals("2") || hit.getId().equals("3")) { - assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(2)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); - assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("title")); - assertThat(hit.getMatchedQueryScore("title"), greaterThan(0f)); - } else { - fail("Unexpected document returned with id " + hit.getId()); - } - } - } - ); - - assertResponse( prepareSearch().setQuery(termsQuery("title", "title1", "title2", "title3").queryName("title")) - .setPostFilter(matchQuery("name", "test").queryName("name")), - response -> { - assertHitCount(response, 3L); - for (SearchHit hit : response.getHits()) { - if (hit.getId().equals("1") || hit.getId().equals("2") || hit.getId().equals("3")) { - assertThat(hit.getMatchedQueriesAndScores().size(), equalTo(2)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("name")); - assertThat(hit.getMatchedQueryScore("name"), greaterThan(0f)); - assertThat(hit.getMatchedQueriesAndScores(), hasKey("title")); - assertThat(hit.getMatchedQueryScore("title"), greaterThan(0f)); - } else { - fail("Unexpected document returned with id " + hit.getId()); - } - } - } + .setPostFilter(matchQuery("name", "test").queryName("name")) ); } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java index 0805d0f366b0f..36580ebda8aee 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java @@ -97,6 +97,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailuresAndResponse; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNotHighlighted; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponses; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; @@ -596,40 +597,24 @@ public void testSourceLookupHighlightingUsingPostingsHighlighter() throws Except } indexRandom(true, indexRequestBuilders); - assertResponse( + assertResponses(response -> { + for (int i = 0; i < indexRequestBuilders.length; i++) { + assertHighlight( + response, + i, + "title", + 0, + equalTo("This is a test on the highlighting bug present in elasticsearch. Hopefully it works.") + ); + assertHighlight(response, i, "title", 1, 2, equalTo("This is the second bug to perform highlighting on.")); + } + }, prepareSearch().setQuery(matchQuery("title", "bug")) // asking for the whole field to be highlighted .highlighter(new HighlightBuilder().field("title", -1, 0)), - response -> { - for (int i = 0; i < indexRequestBuilders.length; i++) { - assertHighlight( - response, - i, - "title", - 0, - equalTo("This is a test on the highlighting bug present in elasticsearch. Hopefully it works.") - ); - assertHighlight(response, i, "title", 1, 2, equalTo("This is the second bug to perform highlighting on.")); - } - } - ); - - assertResponse( prepareSearch().setQuery(matchQuery("title", "bug")) // sentences will be generated out of each value - .highlighter(new HighlightBuilder().field("title")), - response -> { - for (int i = 0; i < indexRequestBuilders.length; i++) { - assertHighlight( - response, - i, - "title", - 0, - equalTo("This is a test on the highlighting bug present in elasticsearch. Hopefully it works.") - ); - assertHighlight(response, i, "title", 1, 2, equalTo("This is the second bug to perform highlighting on.")); - } - } + .highlighter(new HighlightBuilder().field("title")) ); assertResponse( @@ -792,27 +777,31 @@ public void testPlainHighlighterOrder() throws Exception { refresh(); { - // fragments should be in order of appearance by default - SearchSourceBuilder source = searchSource().query(matchQuery("field1", "brown dog")) - .highlighter(highlight().highlighterType("plain").field("field1").preTags("").postTags("").fragmentSize(25)); - - assertResponse(prepareSearch("test").setSource(source), response -> { - - assertHighlight(response, 0, "field1", 0, 3, equalTo("The quick brown fox")); - assertHighlight(response, 0, "field1", 1, 3, equalTo(" jumps over the lazy brown dog")); - assertHighlight(response, 0, "field1", 2, 3, equalTo(" dog doesn't care")); - }); - // lets be explicit about the order - source = searchSource().query(matchQuery("field1", "brown dog")) - .highlighter( - highlight().highlighterType("plain").field("field1").order("none").preTags("").postTags("").fragmentSize(25) - ); - - assertResponse(prepareSearch("test").setSource(source), response -> { + assertResponses(response -> { assertHighlight(response, 0, "field1", 0, 3, equalTo("The quick brown fox")); assertHighlight(response, 0, "field1", 1, 3, equalTo(" jumps over the lazy brown dog")); assertHighlight(response, 0, "field1", 2, 3, equalTo(" dog doesn't care")); - }); + }, + // fragments should be in order of appearance by default + prepareSearch("test").setSource( + searchSource().query(matchQuery("field1", "brown dog")) + .highlighter( + highlight().highlighterType("plain").field("field1").preTags("").postTags("").fragmentSize(25) + ) + ), + // lets be explicit about the order + prepareSearch("test").setSource( + searchSource().query(matchQuery("field1", "brown dog")) + .highlighter( + highlight().highlighterType("plain") + .field("field1") + .order("none") + .preTags("") + .postTags("") + .fragmentSize(25) + ) + ) + ); } { // order by score @@ -1701,42 +1690,26 @@ public void testDisableFastVectorHighlighter() throws Exception { } ); - // Using plain highlighter instead of FVH - assertResponse( + assertResponses(response -> { + for (int i = 0; i < indexRequestBuilders.length; i++) { + assertHighlight( + response, + i, + "title", + 0, + 1, + equalTo("This is a test for the workaround for the fast vector highlighting SOLR-3724") + ); + } + }, + // Using plain highlighter instead of FVH prepareSearch().setQuery(matchPhraseQuery("title", "test for the workaround")) .highlighter(new HighlightBuilder().field("title", 50, 1, 10).highlighterType("plain")), - response -> { - for (int i = 0; i < indexRequestBuilders.length; i++) { - assertHighlight( - response, - i, - "title", - 0, - 1, - equalTo("This is a test for the workaround for the fast vector highlighting SOLR-3724") - ); - } - } - ); - - // Using plain highlighter instead of FVH on the field level - assertResponse( + // Using plain highlighter instead of FVH on the field level prepareSearch().setQuery(matchPhraseQuery("title", "test for the workaround")) .highlighter( new HighlightBuilder().field(new HighlightBuilder.Field("title").highlighterType("plain")).highlighterType("plain") - ), - response -> { - for (int i = 0; i < indexRequestBuilders.length; i++) { - assertHighlight( - response, - i, - "title", - 0, - 1, - equalTo("This is a test for the workaround for the fast vector highlighting SOLR-3724") - ); - } - } + ) ); } @@ -1826,44 +1799,29 @@ public void testPlainHighlightDifferentFragmenter() throws Exception { .get(); refresh(); - assertResponse( + assertResponses(response -> { + assertHighlight(response, 0, "tags", 0, equalTo("this is a really long tag i would like to highlight")); + assertHighlight( + response, + 0, + "tags", + 1, + 2, + equalTo("here is another one that is very long tag and has the tag token near the end") + ); + }, prepareSearch("test").setQuery(QueryBuilders.matchPhraseQuery("tags", "long tag")) .highlighter( new HighlightBuilder().field( new HighlightBuilder.Field("tags").highlighterType("plain").fragmentSize(-1).numOfFragments(2).fragmenter("simple") ) ), - response -> { - assertHighlight(response, 0, "tags", 0, equalTo("this is a really long tag i would like to highlight")); - assertHighlight( - response, - 0, - "tags", - 1, - 2, - equalTo("here is another one that is very long tag and has the tag token near the end") - ); - } - ); - - assertResponse( prepareSearch("test").setQuery(QueryBuilders.matchPhraseQuery("tags", "long tag")) .highlighter( new HighlightBuilder().field( new Field("tags").highlighterType("plain").fragmentSize(-1).numOfFragments(2).fragmenter("span") ) - ), - response -> { - assertHighlight(response, 0, "tags", 0, equalTo("this is a really long tag i would like to highlight")); - assertHighlight( - response, - 0, - "tags", - 1, - 2, - equalTo("here is another one that is very long tag and has the tag token near the end") - ); - } + ) ); assertFailures( @@ -3627,15 +3585,16 @@ public void testWithNestedQuery() throws Exception { assertThat(field.fragments()[1].string(), equalTo("cow")); } ); - assertResponse( + assertResponses(response -> { + assertHitCount(response, 1); + HighlightField field = response.getHits().getAt(0).getHighlightFields().get("foo.text"); + assertThat(field.fragments().length, equalTo(1)); + assertThat(field.fragments()[0].string(), equalTo("brown shoes")); + }, prepareSearch().setQuery(nestedQuery("foo", prefixQuery("foo.text", "bro"), ScoreMode.None)) .highlighter(new HighlightBuilder().field(new Field("foo.text").highlighterType(type))), - response -> { - assertHitCount(response, 1); - HighlightField field = response.getHits().getAt(0).getHighlightFields().get("foo.text"); - assertThat(field.fragments().length, equalTo(1)); - assertThat(field.fragments()[0].string(), equalTo("brown shoes")); - } + prepareSearch().setQuery(nestedQuery("foo", matchPhrasePrefixQuery("foo.text", "bro"), ScoreMode.None)) + .highlighter(new HighlightBuilder().field(new Field("foo.text").highlighterType(type))) ); assertResponse( prepareSearch().setQuery(nestedQuery("foo", matchPhraseQuery("foo.text", "brown shoes"), ScoreMode.None)) @@ -3647,16 +3606,6 @@ public void testWithNestedQuery() throws Exception { assertThat(field.fragments()[0].string(), equalTo("brown shoes")); } ); - assertResponse( - prepareSearch().setQuery(nestedQuery("foo", matchPhrasePrefixQuery("foo.text", "bro"), ScoreMode.None)) - .highlighter(new HighlightBuilder().field(new Field("foo.text").highlighterType(type))), - response -> { - assertHitCount(response, 1); - HighlightField field = response.getHits().getAt(0).getHighlightFields().get("foo.text"); - assertThat(field.fragments().length, equalTo(1)); - assertThat(field.fragments()[0].string(), equalTo("brown shoes")); - } - ); } // For unified and fvh highlighters we just check that the nested query is correctly extracted diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/functionscore/QueryRescorerIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/functionscore/QueryRescorerIT.java index 9fed4ead8c248..a7efb2fe0e68b 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/functionscore/QueryRescorerIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/functionscore/QueryRescorerIT.java @@ -69,6 +69,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailuresAndResponse; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponses; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSecondHit; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThirdHit; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasId; @@ -149,33 +150,24 @@ public void testRescorePhrase() throws Exception { 5 ), response -> { - assertThat(response.getHits().getTotalHits().value(), equalTo(3L)); + assertHitCount(response, 3); assertThat(response.getHits().getMaxScore(), equalTo(response.getHits().getHits()[0].getScore())); - assertThat(response.getHits().getHits()[0].getId(), equalTo("1")); - assertThat(response.getHits().getHits()[1].getId(), equalTo("3")); - assertThat(response.getHits().getHits()[2].getId(), equalTo("2")); + assertFirstHit(response, hasId("1")); + assertSecondHit(response, hasId("3")); + assertThirdHit(response, hasId("2")); } ); - assertResponse( + assertResponses(response -> { + assertHitCount(response, 3); + assertThat(response.getHits().getMaxScore(), equalTo(response.getHits().getHits()[0].getScore())); + assertFirstHit(response, hasId("1")); + assertSecondHit(response, hasId("2")); + assertThirdHit(response, hasId("3")); + }, prepareSearch().setQuery(QueryBuilders.matchQuery("field1", "the quick brown").operator(Operator.OR)) .setRescorer(new QueryRescorerBuilder(matchPhraseQuery("field1", "the quick brown").slop(3)), 5), - response -> { - assertHitCount(response, 3); - assertFirstHit(response, hasId("1")); - assertSecondHit(response, hasId("2")); - assertThirdHit(response, hasId("3")); - } - ); - assertResponse( prepareSearch().setQuery(QueryBuilders.matchQuery("field1", "the quick brown").operator(Operator.OR)) - .setRescorer(new QueryRescorerBuilder(matchPhraseQuery("field1", "the quick brown")), 5), - response -> { - assertHitCount(response, 3); - assertThat(response.getHits().getMaxScore(), equalTo(response.getHits().getHits()[0].getScore())); - assertFirstHit(response, hasId("1")); - assertSecondHit(response, hasId("2")); - assertThirdHit(response, hasId("3")); - } + .setRescorer(new QueryRescorerBuilder(matchPhraseQuery("field1", "the quick brown")), 5) ); } @@ -212,7 +204,15 @@ public void testMoreDocs() throws Exception { prepareIndex("test").setId("11").setSource("field1", "2st street boston massachusetts").get(); prepareIndex("test").setId("12").setSource("field1", "3st street boston massachusetts").get(); indicesAdmin().prepareRefresh("test").get(); - assertResponse( + + assertResponses(response -> { + assertThat(response.getHits().getHits().length, equalTo(5)); + assertHitCount(response, 9); + assertThat(response.getHits().getMaxScore(), equalTo(response.getHits().getHits()[0].getScore())); + assertFirstHit(response, hasId("2")); + assertSecondHit(response, hasId("6")); + assertThirdHit(response, hasId("3")); + }, prepareSearch().setQuery(QueryBuilders.matchQuery("field1", "lexington avenue massachusetts").operator(Operator.OR)) .setFrom(0) .setSize(5) @@ -221,16 +221,6 @@ public void testMoreDocs() throws Exception { .setRescoreQueryWeight(2.0f), 20 ), - response -> { - assertThat(response.getHits().getHits().length, equalTo(5)); - assertHitCount(response, 9); - assertFirstHit(response, hasId("2")); - assertSecondHit(response, hasId("6")); - assertThirdHit(response, hasId("3")); - } - ); - - assertResponse( prepareSearch().setQuery(QueryBuilders.matchQuery("field1", "lexington avenue massachusetts").operator(Operator.OR)) .setFrom(0) .setSize(5) @@ -239,15 +229,7 @@ public void testMoreDocs() throws Exception { new QueryRescorerBuilder(matchPhraseQuery("field1", "lexington avenue massachusetts").slop(3)).setQueryWeight(0.6f) .setRescoreQueryWeight(2.0f), 20 - ), - response -> { - assertThat(response.getHits().getHits().length, equalTo(5)); - assertHitCount(response, 9); - assertThat(response.getHits().getMaxScore(), equalTo(response.getHits().getHits()[0].getScore())); - assertFirstHit(response, hasId("2")); - assertSecondHit(response, hasId("6")); - assertThirdHit(response, hasId("3")); - } + ) ); // Make sure non-zero from works: assertResponse( @@ -465,7 +447,8 @@ public void testEquivalence() throws Exception { .setFrom(0) .setSize(resultSize), plain -> { - assertResponse( + assertResponses( + rescored -> assertEquivalent(query, plain, rescored), prepareSearch().setSearchType(SearchType.QUERY_THEN_FETCH) .setPreference("test") // ensure we hit the same shards for tie-breaking .setQuery(QueryBuilders.matchQuery("field1", query).operator(Operator.OR)) @@ -478,10 +461,6 @@ public void testEquivalence() throws Exception { .setRescoreQueryWeight(0.0f), rescoreWindow ), - rescored -> assertEquivalent(query, plain, rescored) - ); // check equivalence - - assertResponse( prepareSearch().setSearchType(SearchType.QUERY_THEN_FETCH) .setPreference("test") // ensure we hit the same shards for tie-breaking .setQuery(QueryBuilders.matchQuery("field1", query).operator(Operator.OR)) @@ -492,8 +471,7 @@ public void testEquivalence() throws Exception { .setQueryWeight(1.0f) .setRescoreQueryWeight(1.0f), rescoreWindow - ), - rescored -> assertEquivalent(query, plain, rescored) + ) ); // check equivalence } ); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java index 3f6f7af56eb08..69a9fd7fdd4c7 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/query/MultiMatchQueryIT.java @@ -302,27 +302,20 @@ public void testDefaults() throws ExecutionException, InterruptedException { ), response -> assertFirstHit(response, hasId("theother")) ); - assertResponse( + assertResponses(response -> { + assertHitCount(response, 1L); + assertFirstHit(response, hasId("theone")); + }, prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america", "full_name", "first_name", "last_name", "category").operator(Operator.AND).type(type) ) ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("theone")); - } - ); - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america", "full_name", "first_name", "last_name", "category").operator(Operator.AND).type(type) ) - ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("theone")); - } + ) ); } @@ -630,7 +623,10 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException response -> assertFirstHit(response, hasId("theother")) ); - assertResponse( + assertResponses(response -> { + assertHitCount(response, 1L); + assertFirstHit(response, hasId("theone")); + }, prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america", "full_name", "first_name", "last_name", "category").type( @@ -638,12 +634,6 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException ).operator(Operator.AND) ) ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("theone")); - } - ); - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america 15", "full_name", "first_name", "last_name", "category", "skill").type( @@ -651,12 +641,6 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException ).analyzer("category").lenient(true).operator(Operator.AND) ) ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("theone")); - } - ); - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america 15", "full_name", "first_name", "last_name", "category", "skill", "int-field").type( @@ -664,25 +648,17 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException ).analyzer("category").lenient(true).operator(Operator.AND) ) ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("theone")); - } - ); - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america 15", "skill", "full_name", "first_name", "last_name", "category", "int-field").type( MultiMatchQueryBuilder.Type.CROSS_FIELDS ).analyzer("category").lenient(true).operator(Operator.AND) ) - ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("theone")); - } + ) ); - assertResponse( + + assertResponses( + response -> assertFirstHit(response, hasId("theone")), prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america 15", "first_name", "last_name", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS) @@ -690,71 +666,42 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException .analyzer("category") ) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType(multiMatchQuery("15", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS).analyzer("category")) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType(multiMatchQuery("25 15", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS).analyzer("category")) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("25 15", "int-field", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS).analyzer("category") ) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("25 15", "first_name", "int-field", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS) .analyzer("category") ) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("25 15", "int-field", "skill", "first_name").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS) .analyzer("category") ) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("25 15", "int-field", "first_name", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS) .analyzer("category") ) ), - response -> assertFirstHit(response, hasId("theone")) - ); - - assertResponse( prepareSearch("test").setQuery( randomizeType( multiMatchQuery("captain america marvel hero", "first_name", "last_name", "category").type( MultiMatchQueryBuilder.Type.CROSS_FIELDS ).analyzer("category").operator(Operator.OR) ) - ), - response -> assertFirstHit(response, hasId("theone")) + ) ); // test group based on analyzer -- all fields are grouped into a cross field search @@ -771,6 +718,7 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException assertFirstHit(response, hasId("theone")); } ); + // counter example assertHitCount( 0L, @@ -840,33 +788,26 @@ public void testCrossFieldMode() throws ExecutionException, InterruptedException randomizeType(multiMatchQuery("15", "int-field", "first_name", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS)) ) ); - assertResponse( + assertResponses(response -> { + /* + * Doesn't find the one because "alpha 15" isn't a number and we don't + * break on spaces. + */ + assertHitCount(response, 1L); + assertFirstHit(response, hasId("ultimate1")); + }, prepareSearch("test").setQuery( randomizeType( multiMatchQuery("alpha 15", "first_name", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS).lenient(true) ) ), - response -> { - /* - * Doesn't find the one because "alpha 15" isn't a number and we don't - * break on spaces. - */ - assertHitCount(response, 1L); - assertFirstHit(response, hasId("ultimate1")); - } - ); - // Lenient wasn't always properly lenient with two numeric fields - assertResponse( + // Lenient wasn't always properly lenient with two numeric fields prepareSearch("test").setQuery( randomizeType( multiMatchQuery("alpha 15", "int-field", "first_name", "skill").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS) .lenient(true) ) - ), - response -> { - assertHitCount(response, 1L); - assertFirstHit(response, hasId("ultimate1")); - } + ) ); // Check that cross fields works with date fields assertResponse(