Fix GeoIpDownloaderIT.testInvalidTimestamp (#137663)

The GeoIP 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.

See #128284 (comment) for a detailed investigation.

Closes #128284

Author: nielsbauman

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_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<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

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