Network: Remove http.enabled setting

docs/CHANGELOG.asciidoc

@@ -16,6 +16,8 @@
 
 === Breaking Java Changes
 
+* <<remove-http-enabled, Removed `http.enabled` setting>> ({pull}29601[#29601])
+
 === Deprecations
 
 === New Features

docs/reference/migration/migrate_7_0/settings.asciidoc

@@ -25,3 +25,10 @@
   the system property `es.thread_pool.write.use_bulk_as_display_name` was
   available to keep the display output in APIs as `bulk` instead of `write`.
   These fallback settings and this system property have been removed.
+
+[[remove-http-enabled]]
+==== Http enabled setting removed
+
+The setting `http.enabled` previously allowed disabling binding to http, only allowing
+use of the transport client. This setting has been removed, as the transport client
+will be removed in the future, thus requiring http to always be enabled.

docs/reference/modules/http.asciidoc

@@ -110,16 +110,3 @@ client HTTP responses, defaults to unbounded.
 
 It also uses the common
 <<modules-network,network settings>>.
-
-[float]
-=== Disable HTTP
-
-The http module can be completely disabled and not started by setting
-`http.enabled` to `false`. Elasticsearch nodes (and Java clients) communicate
-internally using the <<modules-transport,transport interface>>, not HTTP. It
-might make  sense to disable the `http` layer entirely on nodes which are not
-meant to serve REST requests directly. For instance, you could disable HTTP on
-<<modules-node,data-only nodes>> if you also have
-<<modules-node,client nodes>> which are intended to serve all REST requests.
-Be aware, however, that you will not be able to send any REST requests (eg to
-retrieve node stats) directly to nodes which have HTTP disabled.

modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java

@@ -75,10 +75,14 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
             ReindexPlugin.class);
     }
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected Settings nodeSettings() {
         Settings.Builder settings = Settings.builder().put(super.nodeSettings());
-        settings.put(NetworkModule.HTTP_ENABLED.getKey(), true);
         // Whitelist reindexing from the http host we're going to use
         settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "127.0.0.1:*");
         settings.put(NetworkModule.HTTP_TYPE_KEY, Netty4Plugin.NETTY_HTTP_TRANSPORT_NAME);

modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java

@@ -91,10 +91,13 @@ protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder().put(super.nodeSettings(nodeOrdinal)).put(nodeSettings()).build();
     }
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable HTTP so we can test retries on reindex from remote; in this case the "remote" cluster is just this cluster
+    }
+
     final Settings nodeSettings() {
         return Settings.builder()
-                // enable HTTP so we can test retries on reindex from remote; in this case the "remote" cluster is just this cluster
-                .put(NetworkModule.HTTP_ENABLED.getKey(), true)
                 // whitelist reindexing from the HTTP host we're going to use
                 .put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "127.0.0.1:*")
                 .build();

modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpRequestSizeLimitIT.java

@@ -50,11 +50,15 @@ public class Netty4HttpRequestSizeLimitIT extends ESNetty4IntegTestCase {
 
     private static final ByteSizeValue LIMIT = new ByteSizeValue(2, ByteSizeUnit.KB);
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
-            .put(NetworkModule.HTTP_ENABLED.getKey(), true)
             .put(HierarchyCircuitBreakerService.IN_FLIGHT_REQUESTS_CIRCUIT_BREAKER_LIMIT_SETTING.getKey(), LIMIT)
             .build();
     }

modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4PipeliningDisabledIT.java

@@ -38,11 +38,15 @@
 @ClusterScope(scope = Scope.TEST, supportsDedicatedMasters = false, numDataNodes = 1)
 public class Netty4PipeliningDisabledIT extends ESNetty4IntegTestCase {
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
-            .put(NetworkModule.HTTP_ENABLED.getKey(), true)
             .put("http.pipelining", false)
             .build();
     }

modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4PipeliningEnabledIT.java

@@ -37,11 +37,15 @@
 @ClusterScope(scope = Scope.TEST, supportsDedicatedMasters = false, numDataNodes = 1)
 public class Netty4PipeliningEnabledIT extends ESNetty4IntegTestCase {
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
             .put(super.nodeSettings(nodeOrdinal))
-            .put(NetworkModule.HTTP_ENABLED.getKey(), true)
             .put("http.pipelining", true)
             .build();
     }

qa/smoke-test-http/src/test/java/org/elasticsearch/http/ContextAndHeaderTransportIT.java

@@ -90,7 +90,6 @@ public class ContextAndHeaderTransportIT extends HttpSmokeTestCase {
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
-                .put(NetworkModule.HTTP_ENABLED.getKey(), true)
                 .build();
     }
 

qa/smoke-test-http/src/test/java/org/elasticsearch/http/CorsRegexIT.java

@@ -49,7 +49,6 @@ protected Settings nodeSettings(int nodeOrdinal) {
                 .put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true)
                 .put(SETTING_CORS_ALLOW_METHODS.getKey(), "get, options, post")
                 .put(SETTING_CORS_ENABLED.getKey(), true)
-                .put(NetworkModule.HTTP_ENABLED.getKey(), true)
                 .build();
     }
 

qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java

@@ -57,11 +57,15 @@
  */
 public class DeprecationHttpIT extends HttpSmokeTestCase {
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
-                .put("force.http.enabled", true)
                 // change values of deprecated settings so that accessing them is logged
                 .put(TEST_DEPRECATED_SETTING_TRUE1.getKey(), ! TEST_DEPRECATED_SETTING_TRUE1.getDefault(Settings.EMPTY))
                 .put(TEST_DEPRECATED_SETTING_TRUE2.getKey(), ! TEST_DEPRECATED_SETTING_TRUE2.getDefault(Settings.EMPTY))

qa/smoke-test-http/src/test/java/org/elasticsearch/http/DetailedErrorsDisabledIT.java

@@ -38,12 +38,12 @@
  */
 @ClusterScope(scope = Scope.TEST, supportsDedicatedMasters = false, numDataNodes = 1)
 public class DetailedErrorsDisabledIT extends HttpSmokeTestCase {
+
     // Build our cluster settings
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
-                .put(NetworkModule.HTTP_ENABLED.getKey(), true)
                 .put(HttpTransportSettings.SETTING_HTTP_DETAILED_ERRORS_ENABLED.getKey(), false)
                 .build();
     }

qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpSmokeTestCase.java

@@ -55,13 +55,17 @@ private static String getTypeKey(Class<? extends Plugin> clazz) {
         }
     }
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
                 .put(NetworkModule.TRANSPORT_TYPE_KEY, nodeTransportTypeKey)
-                .put(NetworkModule.HTTP_TYPE_KEY, nodeHttpTypeKey)
-                .put(NetworkModule.HTTP_ENABLED.getKey(), true).build();
+                .put(NetworkModule.HTTP_TYPE_KEY, nodeHttpTypeKey).build();
     }
 
     @Override

qa/smoke-test-http/src/test/java/org/elasticsearch/http/ResponseHeaderPluginIT.java

@@ -37,12 +37,10 @@
  */
 @ClusterScope(scope = Scope.SUITE, supportsDedicatedMasters = false, numDataNodes = 1)
 public class ResponseHeaderPluginIT extends HttpSmokeTestCase {
+
     @Override
-    protected Settings nodeSettings(int nodeOrdinal) {
-        return Settings.builder()
-                .put(super.nodeSettings(nodeOrdinal))
-                .put("force.http.enabled", true)
-                .build();
+    protected boolean addMockHttpTransport() {
+        return false; // enable http
     }
 
     @Override

server/src/main/java/org/elasticsearch/common/network/NetworkModule.java

@@ -71,8 +71,6 @@ public final class NetworkModule {
             Property.NodeScope);
     public static final Setting<String> HTTP_DEFAULT_TYPE_SETTING = Setting.simpleString(HTTP_TYPE_DEFAULT_KEY, Property.NodeScope);
     public static final Setting<String> HTTP_TYPE_SETTING = Setting.simpleString(HTTP_TYPE_KEY, Property.NodeScope);
-    public static final Setting<Boolean> HTTP_ENABLED = Setting.boolSetting("http.enabled", true,
-        Property.NodeScope, Property.Deprecated);
     public static final Setting<String> TRANSPORT_TYPE_SETTING = Setting.simpleString(TRANSPORT_TYPE_KEY, Property.NodeScope);
 
     private final Settings settings;
@@ -117,9 +115,9 @@ public NetworkModule(Settings settings, boolean transportClient, List<NetworkPlu
         this.settings = settings;
         this.transportClient = transportClient;
         for (NetworkPlugin plugin : plugins) {
-            if (transportClient == false && HTTP_ENABLED.get(settings)) {
-                Map<String, Supplier<HttpServerTransport>> httpTransportFactory = plugin.getHttpTransports(settings, threadPool, bigArrays,
-                    circuitBreakerService, namedWriteableRegistry, xContentRegistry, networkService, dispatcher);
+            Map<String, Supplier<HttpServerTransport>> httpTransportFactory = plugin.getHttpTransports(settings, threadPool, bigArrays,
+                circuitBreakerService, namedWriteableRegistry, xContentRegistry, networkService, dispatcher);
+            if (transportClient == false) {
                 for (Map.Entry<String, Supplier<HttpServerTransport>> entry : httpTransportFactory.entrySet()) {
                     registerHttpTransport(entry.getKey(), entry.getValue());
                 }
@@ -197,10 +195,6 @@ public Supplier<HttpServerTransport> getHttpServerTransportSupplier() {
         return factory;
     }
 
-    public boolean isHttpEnabled() {
-        return transportClient == false && HTTP_ENABLED.get(settings);
-    }
-
     public Supplier<Transport> getTransportSupplier() {
         final String name;
         if (TRANSPORT_TYPE_SETTING.exists(settings)) {

server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -219,7 +219,6 @@ public void apply(Settings value, Settings current, Settings previous) {
                     GatewayService.RECOVER_AFTER_MASTER_NODES_SETTING,
                     GatewayService.RECOVER_AFTER_NODES_SETTING,
                     GatewayService.RECOVER_AFTER_TIME_SETTING,
-                    NetworkModule.HTTP_ENABLED,
                     NetworkModule.HTTP_DEFAULT_TYPE_SETTING,
                     NetworkModule.TRANSPORT_DEFAULT_TYPE_SETTING,
                     NetworkModule.HTTP_TYPE_SETTING,

server/src/main/java/org/elasticsearch/node/Node.java

@@ -440,19 +440,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
             final ResponseCollectorService responseCollectorService = new ResponseCollectorService(this.settings, clusterService);
             final SearchTransportService searchTransportService =  new SearchTransportService(settings, transportService,
                 SearchExecutionStatsCollector.makeWrapper(responseCollectorService));
-            final Consumer<Binder> httpBind;
-            final HttpServerTransport httpServerTransport;
-            if (networkModule.isHttpEnabled()) {
-                httpServerTransport = networkModule.getHttpServerTransportSupplier().get();
-                httpBind = b -> {
-                    b.bind(HttpServerTransport.class).toInstance(httpServerTransport);
-                };
-            } else {
-                httpBind = b -> {
-                    b.bind(HttpServerTransport.class).toProvider(Providers.of(null));
-                };
-                httpServerTransport = null;
-            }
+            final HttpServerTransport httpServerTransport = newHttpTransport(networkModule);
 
             final DiscoveryModule discoveryModule = new DiscoveryModule(this.settings, threadPool, transportService, namedWriteableRegistry,
                 networkService, clusterService.getMasterService(), clusterService.getClusterApplierService(),
@@ -519,7 +507,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
                         b.bind(PeerRecoveryTargetService.class).toInstance(new PeerRecoveryTargetService(settings, threadPool,
                                 transportService, recoverySettings, clusterService));
                     }
-                    httpBind.accept(b);
+                    b.bind(HttpServerTransport.class).toInstance(httpServerTransport);
                     pluginComponents.stream().forEach(p -> b.bind((Class) p.getClass()).toInstance(p));
                     b.bind(PersistentTasksService.class).toInstance(persistentTasksService);
                     b.bind(PersistentTasksClusterService.class).toInstance(persistentTasksClusterService);
@@ -541,10 +529,8 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
             client.initialize(injector.getInstance(new Key<Map<GenericAction, TransportAction>>() {}),
                     () -> clusterService.localNode().getId(), transportService.getRemoteClusterService());
 
-            if (NetworkModule.HTTP_ENABLED.get(settings)) {
-                logger.debug("initializing HTTP handlers ...");
-                actionModule.initRestHandlers(() -> clusterService.state().nodes());
-            }
+            logger.debug("initializing HTTP handlers ...");
+            actionModule.initRestHandlers(() -> clusterService.state().nodes());
             logger.info("initialized");
 
             success = true;
@@ -704,18 +690,13 @@ public void onTimeout(TimeValue timeout) {
             }
         }
 
-
-        if (NetworkModule.HTTP_ENABLED.get(settings)) {
-            injector.getInstance(HttpServerTransport.class).start();
-        }
+        injector.getInstance(HttpServerTransport.class).start();
 
         if (WRITE_PORTS_FILE_SETTING.get(settings)) {
-            if (NetworkModule.HTTP_ENABLED.get(settings)) {
-                HttpServerTransport http = injector.getInstance(HttpServerTransport.class);
-                writePortsFile("http", http.boundAddress());
-            }
             TransportService transport = injector.getInstance(TransportService.class);
             writePortsFile("transport", transport.boundAddress());
+            HttpServerTransport http = injector.getInstance(HttpServerTransport.class);
+            writePortsFile("http", http.boundAddress());
         }
 
         logger.info("started");
@@ -733,9 +714,7 @@ private Node stop() {
         logger.info("stopping ...");
 
         injector.getInstance(ResourceWatcherService.class).stop();
-        if (NetworkModule.HTTP_ENABLED.get(settings)) {
-            injector.getInstance(HttpServerTransport.class).stop();
-        }
+        injector.getInstance(HttpServerTransport.class).stop();
 
         injector.getInstance(SnapshotsService.class).stop();
         injector.getInstance(SnapshotShardsService.class).stop();
@@ -781,9 +760,7 @@ public synchronized void close() throws IOException {
         toClose.add(() -> stopWatch.start("node_service"));
         toClose.add(nodeService);
         toClose.add(() -> stopWatch.stop().start("http"));
-        if (NetworkModule.HTTP_ENABLED.get(settings)) {
-            toClose.add(injector.getInstance(HttpServerTransport.class));
-        }
+        toClose.add(injector.getInstance(HttpServerTransport.class));
         toClose.add(() -> stopWatch.stop().start("snapshot_service"));
         toClose.add(injector.getInstance(SnapshotsService.class));
         toClose.add(injector.getInstance(SnapshotShardsService.class));
@@ -963,6 +940,11 @@ protected ClusterInfoService newClusterInfoService(Settings settings, ClusterSer
         return new InternalClusterInfoService(settings, clusterService, threadPool, client, listeners);
     }
 
+    /** Constructs a {@link org.elasticsearch.http.HttpServerTransport} which may be mocked for tests. */
+    protected HttpServerTransport newHttpTransport(NetworkModule networkModule) {
+        return networkModule.getHttpServerTransportSupplier().get();
+    }
+
     private static class LocalNodeFactory implements Function<BoundTransportAddress, DiscoveryNode> {
         private final SetOnce<DiscoveryNode> localNode = new SetOnce<>();
         private final String persistentNodeId;

server/src/main/java/org/elasticsearch/transport/RemoteClusterConnection.java

@@ -633,6 +633,7 @@ public void handleResponse(NodesInfoResponse response) {
                     }
 
                     if (httpAddresses.size() < maxNumRemoteConnections) {
+                        // TODO: remove this when master is 8.0, since 7.0+ nodes always have http enabled
                         // just in case non of the connected nodes have http enabled we get other http enabled nodes instead.
                         for (NodeInfo info : response.getNodes()) {
                             if (nodePredicate.test(info.getNode()) && info.getHttp() != null) {

server/src/test/java/org/elasticsearch/client/transport/TransportClientIT.java

@@ -32,6 +32,7 @@
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.elasticsearch.test.ESIntegTestCase.Scope;
+import org.elasticsearch.test.MockHttpTransport;
 import org.elasticsearch.test.discovery.TestZenDiscovery;
 import org.elasticsearch.transport.MockTransportClient;
 import org.elasticsearch.transport.TransportService;
@@ -62,10 +63,10 @@ public void testNodeVersionIsUpdated() throws IOException, NodeValidationExcepti
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir())
                 .put("node.name", "testNodeVersionIsUpdated")
                 .put("transport.type", getTestTransportType())
-                .put(NetworkModule.HTTP_ENABLED.getKey(), false)
                 .put(Node.NODE_DATA_SETTING.getKey(), false)
                 .put("cluster.name", "foobar")
-                .build(), Arrays.asList(getTestTransportPlugin(), TestZenDiscovery.TestPlugin.class)).start()) {
+                .build(), Arrays.asList(getTestTransportPlugin(), TestZenDiscovery.TestPlugin.class,
+                                        MockHttpTransport.TestPlugin.class)).start()) {
             TransportAddress transportAddress = node.injector().getInstance(TransportService.class).boundAddress().publishAddress();
             client.addTransportAddress(transportAddress);
             // since we force transport clients there has to be one node started that we connect to.