Enable GeoIP downloader by default (#71505)

This change enables GeoIP downloader by default.
It removes feature flag but adds flag that is used by tests to disable it again (as we don't want to hammer GeoIP database service with every test cluster we spin up).

Relates to #68920

build.gradle

@@ -21,6 +21,7 @@ import org.gradle.util.DistributionLocator
 import org.gradle.util.GradleVersion
 import static org.elasticsearch.gradle.util.GradleUtils.maybeConfigure
 import org.gradle.plugins.ide.eclipse.model.ProjectDependency
+import org.elasticsearch.gradle.testclusters.TestClustersPlugin
 
 plugins {
   id 'lifecycle-base'
@@ -515,3 +516,11 @@ subprojects {
     }
   }
 }
+
+subprojects { Project subproj ->
+  plugins.withType(TestClustersPlugin).whenPluginAdded {
+    testClusters.all {
+      systemProperty "geoip.downloader.enabled.default", "false"
+    }
+  }
+}

client/rest-high-level/build.gradle

@@ -88,8 +88,6 @@ tasks.named("check").configure {
 testClusters.all {
   testDistribution = 'DEFAULT'
   systemProperty 'es.scripting.update.ctx_in_params', 'false'
-  systemProperty 'es.geoip_v2_feature_flag_enabled', 'true'
-  setting 'geoip.downloader.enabled', 'false'
   setting 'reindex.remote.whitelist', '[ "[::1]:*", "127.0.0.1:*" ]'
   setting 'xpack.license.self_generated.type', 'trial'
   setting 'xpack.security.enabled', 'true'

distribution/docker/docker-compose.yml

@@ -16,6 +16,7 @@ services:
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
        - node.store.allow_mmap=false
+       - geoip.downloader.enabled=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
        - xpack.security.http.ssl.enabled=true
@@ -68,6 +69,7 @@ services:
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
        - node.store.allow_mmap=false
+       - geoip.downloader.enabled=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
        - xpack.security.http.ssl.enabled=true

modules/ingest-geoip/build.gradle

@@ -52,17 +52,11 @@ if (useFixture) {
 }
 
 tasks.named("internalClusterTest").configure {
-  systemProperty "es.geoip_v2_feature_flag_enabled", "true"
   if (useFixture) {
     nonInputProperties.systemProperty "geoip_endpoint", "${-> fixtureAddress()}"
   }
 }
 
-testClusters.all {
-  systemProperty "es.geoip_v2_feature_flag_enabled", "true"
-  setting "geoip.downloader.enabled", "false"
-}
-
 tasks.register("copyDefaultGeoIp2DatabaseFiles", Copy) {
   from { zipTree(configurations.testCompileClasspath.files.find { it.name.contains('geolite2-databases') }) }
   into "${project.buildDir}/ingest-geoip"

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpDownloader.java

@@ -58,8 +58,6 @@ public class GeoIpDownloader extends AllocatedPersistentTask {
 
     private static final Logger logger = LogManager.getLogger(GeoIpDownloader.class);
 
-    public static final boolean GEOIP_V2_FEATURE_FLAG_ENABLED = "true".equals(System.getProperty("es.geoip_v2_feature_flag_enabled"));
-
     public static final Setting<TimeValue> POLL_INTERVAL_SETTING = Setting.timeSetting("geoip.downloader.poll.interval",
         TimeValue.timeValueDays(3), TimeValue.timeValueDays(1), Property.Dynamic, Property.NodeScope);
     public static final Setting<String> ENDPOINT_SETTING = Setting.simpleString("geoip.downloader.endpoint",

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpDownloaderTaskExecutor.java

@@ -30,7 +30,6 @@ import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_DOWNLOADER;
-import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_V2_FEATURE_FLAG_ENABLED;
 
 /**
  * Persistent task executor that is responsible for starting {@link GeoIpDownloader} after task is allocated by master node.
@@ -38,7 +37,8 @@ import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_V2_FEATURE_FL
  */
 public final class GeoIpDownloaderTaskExecutor extends PersistentTasksExecutor<GeoIpTaskParams> implements ClusterStateListener {
 
-    public static final Setting<Boolean> ENABLED_SETTING = Setting.boolSetting("geoip.downloader.enabled", GEOIP_V2_FEATURE_FLAG_ENABLED,
+    private static final boolean ENABLED_DEFAULT = "false".equals(System.getProperty("geoip.downloader.enabled.default")) == false;
+    public static final Setting<Boolean> ENABLED_SETTING = Setting.boolSetting("geoip.downloader.enabled", ENABLED_DEFAULT,
         Setting.Property.Dynamic, Setting.Property.NodeScope);
 
     private static final Logger logger = LogManager.getLogger(GeoIpDownloader.class);

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java

@@ -55,7 +55,6 @@ import org.elasticsearch.watcher.ResourceWatcherService;
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.UncheckedIOException;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -67,7 +66,6 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
 import static org.elasticsearch.ingest.geoip.GeoIpDownloader.DATABASES_INDEX;
 import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_DOWNLOADER;
-import static org.elasticsearch.ingest.geoip.GeoIpDownloader.GEOIP_V2_FEATURE_FLAG_ENABLED;
 
 public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemIndexPlugin, Closeable, PersistentTaskPlugin, ActionPlugin {
     public static final Setting<Long> CACHE_SIZE =
@@ -81,13 +79,8 @@ public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemInd
 
     @Override
     public List<Setting<?>> getSettings() {
-        List<Setting<?>> settings = new ArrayList<>(Arrays.asList(CACHE_SIZE,
-            GeoIpDownloader.ENDPOINT_SETTING,
-            GeoIpDownloader.POLL_INTERVAL_SETTING));
-        if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
-            settings.add(GeoIpDownloaderTaskExecutor.ENABLED_SETTING);
-        }
-        return settings;
+        return Arrays.asList(CACHE_SIZE, GeoIpDownloader.ENDPOINT_SETTING, GeoIpDownloader.POLL_INTERVAL_SETTING,
+            GeoIpDownloaderTaskExecutor.ENABLED_SETTING);
     }
 
     @Override
@@ -119,11 +112,9 @@ public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemInd
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
-        if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
-            geoIpDownloaderTaskExecutor = new GeoIpDownloaderTaskExecutor(client, new HttpClient(), clusterService, threadPool);
-            return List.of(databaseRegistry.get(), geoIpDownloaderTaskExecutor);
-        }
-        return List.of(databaseRegistry.get());
+
+        geoIpDownloaderTaskExecutor = new GeoIpDownloaderTaskExecutor(client, new HttpClient(), clusterService, threadPool);
+        return List.of(databaseRegistry.get(), geoIpDownloaderTaskExecutor);
     }
 
     @Override
@@ -135,19 +126,12 @@ public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemInd
     public List<PersistentTasksExecutor<?>> getPersistentTasksExecutor(ClusterService clusterService, ThreadPool threadPool,
                                                                        Client client, SettingsModule settingsModule,
                                                                        IndexNameExpressionResolver expressionResolver) {
-        if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
-            return List.of(geoIpDownloaderTaskExecutor);
-        } else {
-            return List.of();
-        }
+        return List.of(geoIpDownloaderTaskExecutor);
     }
 
     @Override
     public List<ActionHandler<? extends ActionRequest, ? extends ActionResponse>> getActions() {
-        if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
-            return List.of(new ActionHandler<>(GeoIpDownloaderStatsAction.INSTANCE, GeoIpDownloaderStatsTransportAction.class));
-        }
-        return Collections.emptyList();
+        return List.of(new ActionHandler<>(GeoIpDownloaderStatsAction.INSTANCE, GeoIpDownloaderStatsTransportAction.class));
     }
 
     @Override
@@ -155,10 +139,7 @@ public class IngestGeoIpPlugin extends Plugin implements IngestPlugin, SystemInd
                                              IndexScopedSettings indexScopedSettings, SettingsFilter settingsFilter,
                                              IndexNameExpressionResolver indexNameExpressionResolver,
                                              Supplier<DiscoveryNodes> nodesInCluster) {
-        if (GEOIP_V2_FEATURE_FLAG_ENABLED) {
-            return List.of(new RestGeoIpDownloaderStatsAction());
-        }
-        return Collections.emptyList();
+        return List.of(new RestGeoIpDownloaderStatsAction());
     }
 
     @Override

qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java

@@ -30,6 +30,7 @@ import static org.elasticsearch.packaging.util.FileExistenceMatchers.fileExists;
 import static org.elasticsearch.packaging.util.FileUtils.append;
 import static org.elasticsearch.packaging.util.FileUtils.mv;
 import static org.elasticsearch.packaging.util.FileUtils.rm;
+import static org.elasticsearch.packaging.util.ServerUtils.disableGeoIpDownloader;
 import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -50,6 +51,7 @@ public class ArchiveTests extends PackagingTestCase {
     public void test10Install() throws Exception {
         installation = installArchive(sh, distribution());
         verifyArchiveInstallation(installation, distribution());
+        disableGeoIpDownloader(installation);
     }
 
     public void test20PluginsListWithNoPlugins() throws Exception {

qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java

@@ -86,7 +86,7 @@ public class DockerTests extends PackagingTestCase {
 
     @Before
     public void setupTest() throws IOException {
-        installation = runContainer(distribution());
+        installation = runContainer(distribution(), builder().envVars(Map.of("geoip.downloader.enabled", "false")));
         tempDir = createTempDir(DockerTests.class.getSimpleName());
     }
 

qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java

@@ -42,6 +42,7 @@ import static org.elasticsearch.packaging.util.Packages.assertInstalled;
 import static org.elasticsearch.packaging.util.Packages.assertRemoved;
 import static org.elasticsearch.packaging.util.Packages.installPackage;
 import static org.elasticsearch.packaging.util.Packages.verifyPackageInstallation;
+import static org.elasticsearch.packaging.util.ServerUtils.disableGeoIpDownloader;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
@@ -64,6 +65,8 @@ public class KeystoreManagementTests extends PackagingTestCase {
         installation = installArchive(sh, distribution);
         verifyArchiveInstallation(installation, distribution());
 
+        disableGeoIpDownloader(installation);
+
         final Installation.Executables bin = installation.executables();
         Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
         assertFalse("has-passwd should fail", r.isSuccess());
@@ -78,6 +81,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
         installation = installPackage(sh, distribution);
         assertInstalled(distribution);
         verifyPackageInstallation(installation, distribution, sh);
+        disableGeoIpDownloader(installation);
 
         final Installation.Executables bin = installation.executables();
         Shell.Result r = sh.runIgnoreExitCode(bin.keystoreTool.toString() + " has-passwd");
@@ -91,7 +95,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
     public void test12InstallDockerDistribution() throws Exception {
         assumeTrue(distribution().isDocker());
 
-        installation = Docker.runContainer(distribution());
+        installation = Docker.runContainer(distribution(), builder().envVars(Map.of("geoip.downloader.enabled", "false")));
 
         try {
             waitForPathToExist(installation.config("elasticsearch.keystore"));
@@ -269,7 +273,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         // restart ES with password and mounted keystore
         Map<Path, Path> volumes = Map.of(localKeystoreFile, dockerKeystore);
-        Map<String, String> envVars = Map.of("KEYSTORE_PASSWORD", password);
+        Map<String, String> envVars = Map.of("KEYSTORE_PASSWORD", password, "geoip.downloader.enabled", "false");
         runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
         waitForElasticsearch(installation);
         ServerUtils.runElasticsearchTests();
@@ -297,7 +301,12 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
             // restart ES with password and mounted keystore
             Map<Path, Path> volumes = Map.of(localKeystoreFile, dockerKeystore, tempDir, Path.of("/run/secrets"));
-            Map<String, String> envVars = Map.of("KEYSTORE_PASSWORD_FILE", "/run/secrets/" + passwordFilename);
+            Map<String, String> envVars = Map.of(
+                "KEYSTORE_PASSWORD_FILE",
+                "/run/secrets/" + passwordFilename,
+                "geoip.downloader.enabled",
+                "false"
+            );
 
             runContainer(distribution(), builder().volumes(volumes).envVars(envVars));
 

qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java

@@ -40,6 +40,7 @@ import static org.elasticsearch.packaging.util.Packages.restartElasticsearch;
 import static org.elasticsearch.packaging.util.Packages.verifyPackageInstallation;
 import static org.elasticsearch.packaging.util.Platforms.getOsRelease;
 import static org.elasticsearch.packaging.util.Platforms.isSystemd;
+import static org.elasticsearch.packaging.util.ServerUtils.disableGeoIpDownloader;
 import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
 import static org.elasticsearch.packaging.util.ServerUtils.runElasticsearchTests;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -62,6 +63,7 @@ public class PackageTests extends PackagingTestCase {
         installation = installPackage(sh, distribution());
         assertInstalled(distribution());
         verifyPackageInstallation(installation, distribution(), sh);
+        disableGeoIpDownloader(installation);
     }
 
     public void test20PluginsCommandWhenNoPlugins() {
@@ -213,6 +215,7 @@ public class PackageTests extends PackagingTestCase {
     public void test60Reinstall() throws Exception {
         install();
         assertInstalled(distribution());
+        disableGeoIpDownloader(installation);
         verifyPackageInstallation(installation, distribution(), sh);
 
         remove(distribution());
@@ -223,6 +226,7 @@ public class PackageTests extends PackagingTestCase {
         try {
             install();
             assertInstalled(distribution());
+            disableGeoIpDownloader(installation);
 
             startElasticsearch();
             restartElasticsearch(sh, installation);

qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java

@@ -27,6 +27,7 @@ import static org.elasticsearch.packaging.util.Archives.installArchive;
 import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
 import static org.elasticsearch.packaging.util.FileUtils.append;
 import static org.elasticsearch.packaging.util.FileUtils.mv;
+import static org.elasticsearch.packaging.util.ServerUtils.disableGeoIpDownloader;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 
@@ -93,6 +94,7 @@ public class WindowsServiceTests extends PackagingTestCase {
         installation = installArchive(sh, distribution());
         verifyArchiveInstallation(installation, distribution());
         serviceScript = installation.bin("elasticsearch-service.bat").toString();
+        disableGeoIpDownloader(installation);
     }
 
     public void test11InstallServiceExeMissing() throws IOException {

qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java

@@ -38,9 +38,13 @@ import java.nio.file.Path;
 import java.security.KeyStore;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
+import java.util.Collections;
+import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 
+import static java.nio.file.StandardOpenOption.APPEND;
+import static java.nio.file.StandardOpenOption.CREATE;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 
@@ -246,4 +250,9 @@ public class ServerUtils {
 
         return body;
     }
+
+    public static void disableGeoIpDownloader(Installation installation) throws IOException {
+        List<String> yaml = Collections.singletonList("geoip.downloader.enabled: false");
+        Files.write(installation.config("elasticsearch.yml"), yaml, CREATE, APPEND);
+    }
 }

qa/remote-clusters/docker-compose.yml

@@ -16,6 +16,7 @@ services:
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
        - node.store.allow_mmap=false
+       - geoip.downloader.enabled=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
        - xpack.security.http.ssl.enabled=true
@@ -69,6 +70,7 @@ services:
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
        - node.store.allow_mmap=false
+       - geoip.downloader.enabled=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
        - xpack.security.http.ssl.enabled=true

x-pack/plugin/security/qa/operator-privileges-tests/src/javaRestTest/java/org/elasticsearch/xpack/security/operator/Constants.java

@@ -228,6 +228,7 @@ public class Constants {
         "cluster:monitor/data_frame/stats/get",
         "cluster:monitor/eql/async/status",
         "cluster:monitor/health",
+        "cluster:monitor/ingest/geoip/stats",
         "cluster:monitor/main",
         "cluster:monitor/nodes/hot_threads",
         "cluster:monitor/nodes/info",

x-pack/test/idp-fixture/docker-compose.yml

@@ -15,6 +15,7 @@ services:
       - cluster.routing.allocation.disk.watermark.high=1b
       - cluster.routing.allocation.disk.watermark.flood_stage=1b
       - node.store.allow_mmap=false
+      - geoip.downloader.enabled=false
       - xpack.license.self_generated.type=trial
       - xpack.security.enabled=true
       - xpack.security.http.ssl.enabled=true