Fix GeoIpDownloaderIT.testInvalidTimestamp

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<Class<? extends Plugin>> 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<Path> files = Files.list(geoIpTmpDir)) {
-                    Set<String> 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<IndexSettingProvider> getAdditionalIndexSettingProviders(IndexSettingProvider.Parameters parameters) {
+            return List.of(new GeoIpIndexSettingProvider());
+        }
+    }
+
+    /**
+     * An index setting provider that disables the request cache for the geoip downloader 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<CompressedXContent> combinedTemplateMappings,
+            IndexVersion indexVersion,
+            Settings.Builder additionalSettings
+        ) {
+            if (GeoIpDownloader.GEOIP_DOWNLOADER.equals(indexName)) {
+                additionalSettings.put(IndicesRequestCache.INDEX_CACHE_REQUEST_ENABLED_SETTING.getKey(), false);
+            }
+        }
+    }
 }

muted-tests.yml

@@ -206,9 +206,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