Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network: Remove http.enabled setting #29601

Merged
merged 18 commits into from
May 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ Machine Learning::
* The `max_running_jobs` node property is removed in this release. Use the
`xpack.ml.max_open_jobs` setting instead. For more information, see <<ml-settings>>.

* <<remove-http-enabled, Removed `http.enabled` setting>> ({pull}29601[#29601])

=== Deprecations
Monitoring::
* The `xpack.monitoring.collection.interval` setting can no longer be set to `-1`
to disable monitoring data collection. Use `xpack.monitoring.collection.enabled`
Expand Down
7 changes: 7 additions & 0 deletions docs/reference/migration/migrate_7_0/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
13 changes: 0 additions & 13 deletions docs/reference/modules/http.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
46 changes: 14 additions & 32 deletions server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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();
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
Loading