diff --git a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java index 5423e6b2a917f..ed4822af1de91 100644 --- a/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java +++ b/modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderIT.java @@ -16,18 +16,24 @@ import org.elasticsearch.action.ingest.SimulatePipelineRequest; import org.elasticsearch.action.ingest.SimulatePipelineResponse; import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.metadata.ProjectMetadata; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.IOUtils; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.core.TimeValue; import org.elasticsearch.env.Environment; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettingProvider; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.RangeQueryBuilder; +import org.elasticsearch.indices.IndicesRequestCache; import org.elasticsearch.ingest.AbstractProcessor; import org.elasticsearch.ingest.IngestDocument; import org.elasticsearch.ingest.Processor; @@ -51,6 +57,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.time.Instant; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -69,6 +76,7 @@ import static org.elasticsearch.ingest.IngestPipelineTestUtils.jsonSimulatePipelineRequest; import static org.elasticsearch.ingest.geoip.GeoIpTestUtils.copyDefaultDatabases; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoSearchHits; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.anEmptyMap; @@ -78,7 +86,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -93,7 +100,8 @@ protected Collection> nodePlugins() { ReindexPlugin.class, IngestGeoIpPlugin.class, GeoIpProcessorNonIngestNodeIT.IngestGeoIpSettingsPlugin.class, - NonGeoProcessorsPlugin.class + NonGeoProcessorsPlugin.class, + GeoIpIndexSettingProviderPlugin.class ); } @@ -186,6 +194,11 @@ public void testInvalidTimestamp() throws Exception { } } }); + // We wait for the deletion of the database chunks in the .geoip_databases index. Since the search request cache is disabled by + // GeoIpIndexSettingProvider, we can be sure that all nodes are unable to fetch the deleted chunks from the cache. + logger.info("---> waiting for database chunks to be deleted"); + assertBusy(() -> assertNoSearchHits(prepareSearch(GeoIpDownloader.DATABASES_INDEX).setRequestCache(false))); + logger.info("---> database chunks deleted"); putGeoIpPipeline(); assertBusy(() -> { SimulateDocumentBaseResult result = simulatePipeline(); @@ -204,19 +217,12 @@ public void testInvalidTimestamp() throws Exception { assertFalse(result.getIngestDocument().hasField("ip-asn")); assertFalse(result.getIngestDocument().hasField("ip-country")); }); - updateClusterSettings(Settings.builder().putNull("ingest.geoip.database_validity")); - assertBusy(() -> { - for (Path geoIpTmpDir : geoIpTmpDirs) { - try (Stream files = Files.list(geoIpTmpDir)) { - Set names = files.map(f -> f.getFileName().toString()).collect(Collectors.toSet()); - assertThat( - names, - hasItems("GeoLite2-ASN.mmdb", "GeoLite2-City.mmdb", "GeoLite2-Country.mmdb", "MyCustomGeoLite2-City.mmdb") - ); - } - } - }); - awaitAllNodesDownloadedDatabases(); + // Ideally, we would reset the validity setting to its default value (30d) here and verify that the databases are downloaded again. + // However, this expiration logic assumes that the md5 hashes of the databases change over time, as it deletes all database chunks + // from the .geoip_databases index. If the hashes do not change, no new database chunks are indexed and so the ingest nodes fail + // to download the databases as there are no chunks to download. + // In this test suite, we don't have a way to change the database files served by the fixture, and thus change their md5 hashes. + // Since we assume that in a real world scenario the database files change over time, we are ok with skipping this part of the test. } @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/92888") @@ -855,4 +861,37 @@ public String getType() { return procMap; } } + + /** + * A simple plugin that provides the {@link GeoIpIndexSettingProvider}. + */ + public static final class GeoIpIndexSettingProviderPlugin extends Plugin { + @Override + public Collection getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) { + return List.of(new GeoIpIndexSettingProvider()); + } + } + + /** + * An index setting provider that disables the request cache for the `.geoip_databases` index. + * Since `.geoip_databases` is a system index, we can't configure this setting using the API or index templates. + */ + public static final class GeoIpIndexSettingProvider implements IndexSettingProvider { + @Override + public void provideAdditionalSettings( + String indexName, + String dataStreamName, + IndexMode templateIndexMode, + ProjectMetadata projectMetadata, + Instant resolvedAt, + Settings indexTemplateAndCreateRequestSettings, + List combinedTemplateMappings, + IndexVersion indexVersion, + Settings.Builder additionalSettings + ) { + if (GeoIpDownloader.GEOIP_DOWNLOADER.equals(indexName)) { + additionalSettings.put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), false); + } + } + } } diff --git a/muted-tests.yml b/muted-tests.yml index 4571d1267cd65..a208fd9ba0bf9 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -233,9 +233,6 @@ tests: - class: org.elasticsearch.packaging.test.EnrollmentProcessTests method: test20DockerAutoFormCluster issue: https://github.com/elastic/elasticsearch/issues/128113 -- class: org.elasticsearch.ingest.geoip.GeoIpDownloaderCliIT - method: testInvalidTimestamp - issue: https://github.com/elastic/elasticsearch/issues/128284 - class: org.elasticsearch.packaging.test.TemporaryDirectoryConfigTests method: test21AcceptsCustomPathInDocker issue: https://github.com/elastic/elasticsearch/issues/128114