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

Compatible API header parsing plugin #60516

Closed
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
c6aeb38
draft
pgomulka Jul 27, 2020
e85833e
compatible version from a plugin
pgomulka Jul 27, 2020
7f8b84a
moving compatibility function to a plugin
pgomulka Jul 28, 2020
88598dc
remove unused interface
pgomulka Jul 28, 2020
c009235
suggested changes
jakelandis Jul 28, 2020
dce47ac
cleanup
jakelandis Jul 28, 2020
3470081
more cleanup
jakelandis Jul 28, 2020
72d7b13
Merge pull request #21 from jakelandis/jake-xpack-spi-request
pgomulka Jul 29, 2020
6b06d7f
interface instead of function
pgomulka Jul 29, 2020
5ec8038
testcase for combinations
pgomulka Jul 29, 2020
a45171e
add hascontent argumetn and pass tests
pgomulka Jul 29, 2020
4ccf457
precommit
pgomulka Jul 29, 2020
8770870
RestCompatibility plugin injected into RestRequest
pgomulka Jul 31, 2020
0c6a5f7
Merge branch 'master' into compat_plugin_inside_rest_request
pgomulka Aug 20, 2020
8e834b6
precommit
pgomulka Aug 20, 2020
29fcc81
header validation test
pgomulka Aug 21, 2020
8a0952b
precommit
pgomulka Aug 21, 2020
b3c626e
compatibleWithVersion on RestHandler
pgomulka Aug 21, 2020
8f42c1d
import
pgomulka Aug 21, 2020
924a35e
fake request with a default to current
pgomulka Aug 24, 2020
0e0e590
split plugin interface
pgomulka Sep 1, 2020
51376bb
rename file
pgomulka Sep 1, 2020
03e9a63
remove evaluaiton depends on
pgomulka Sep 1, 2020
e790a5d
remove compatible function fron http server transport
pgomulka Sep 3, 2020
a2bc52f
cleanup
pgomulka Sep 3, 2020
6724dda
add version to channel
pgomulka Sep 3, 2020
3f606a3
fix nullpointer
pgomulka Sep 3, 2020
e834a9c
Apply suggestions from code review
pgomulka Sep 4, 2020
4014a8d
javadoc and exception repalce
pgomulka Sep 7, 2020
1a9339a
Merge branch 'master' into compat_plugin_channel2
pgomulka Sep 7, 2020
541cf6d
remove exception and empty lines
pgomulka Sep 8, 2020
fd34207
Apply suggestions from code review
pgomulka Sep 14, 2020
b3e7654
add testcase to node init
pgomulka Sep 14, 2020
1d5b099
unused import
pgomulka Sep 14, 2020
1bb983b
Update server/src/main/java/org/elasticsearch/node/Node.java
pgomulka Sep 17, 2020
448a8e7
Merge branch 'master' into compat_plugin_inside_rest_request
pgomulka Oct 5, 2020
91011cc
Merge branch 'compat_plugin_inside_rest_request' of github.com:pgomul…
pgomulka Oct 5, 2020
89d50f5
use media type parser
pgomulka Oct 5, 2020
97baf91
spotless
pgomulka Oct 6, 2020
324dd57
Merge branch 'master' into compat_plugin_inside_rest_request
pgomulka Oct 7, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.elasticsearch.http.HttpReadTimeoutException;
import org.elasticsearch.http.HttpServerChannel;
import org.elasticsearch.http.netty4.cors.Netty4CorsHandler;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.SharedGroupFactory;
import org.elasticsearch.transport.NettyAllocator;
Expand Down Expand Up @@ -147,8 +148,8 @@ public class Netty4HttpServerTransport extends AbstractHttpServerTransport {

public Netty4HttpServerTransport(Settings settings, NetworkService networkService, BigArrays bigArrays, ThreadPool threadPool,
NamedXContentRegistry xContentRegistry, Dispatcher dispatcher, ClusterSettings clusterSettings,
SharedGroupFactory sharedGroupFactory) {
super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher, clusterSettings);
SharedGroupFactory sharedGroupFactory, RestCompatibility restCompatibleFunction) {
super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher, clusterSettings, restCompatibleFunction);
Netty4Utils.setAvailableProcessors(EsExecutors.NODE_PROCESSORS_SETTING.get(settings));
this.sharedGroupFactory = sharedGroupFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.plugins.NetworkPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.netty4.Netty4Transport;

Expand Down Expand Up @@ -90,10 +91,11 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports(Settings set
NamedXContentRegistry xContentRegistry,
NetworkService networkService,
HttpServerTransport.Dispatcher dispatcher,
ClusterSettings clusterSettings) {
ClusterSettings clusterSettings,
RestCompatibility restCompatibleFunction) {
return Collections.singletonMap(NETTY_HTTP_TRANSPORT_NAME,
() -> new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher,
clusterSettings, getSharedGroupFactory(settings)));
clusterSettings, getSharedGroupFactory(settings), restCompatibleFunction));
}

private SharedGroupFactory getSharedGroupFactory(Settings settings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.http.HttpTransportSettings;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -91,7 +92,7 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext,
Settings settings = Settings.builder().put(HttpTransportSettings.SETTING_HTTP_PORT.getKey(), getPortRange()).build();
try (HttpServerTransport httpServerTransport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool,
xContentRegistry(), dispatcher, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new SharedGroupFactory(Settings.EMPTY))) {
new SharedGroupFactory(Settings.EMPTY), RestCompatibility.CURRENT_VERSION)) {
httpServerTransport.start();
final TransportAddress transportAddress = randomFrom(httpServerTransport.boundAddress().boundAddresses());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.http.NullDispatcher;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.TestThreadPool;
Expand Down Expand Up @@ -120,7 +121,7 @@ class CustomNettyHttpServerTransport extends Netty4HttpServerTransport {
Netty4HttpServerPipeliningTests.this.bigArrays,
Netty4HttpServerPipeliningTests.this.threadPool,
xContentRegistry(), new NullDispatcher(), new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new SharedGroupFactory(settings));
new SharedGroupFactory(settings), RestCompatibility.CURRENT_VERSION);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.elasticsearch.http.HttpTransportSettings;
import org.elasticsearch.http.NullDispatcher;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -170,7 +171,7 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext,
}
};
try (Netty4HttpServerTransport transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool,
xContentRegistry(), dispatcher, clusterSettings, new SharedGroupFactory(settings))) {
xContentRegistry(), dispatcher, clusterSettings, new SharedGroupFactory(settings), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());
try (Netty4HttpClient client = new Netty4HttpClient()) {
Expand Down Expand Up @@ -204,15 +205,17 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext,
public void testBindUnavailableAddress() {
Settings initialSettings = createSettings();
try (Netty4HttpServerTransport transport = new Netty4HttpServerTransport(initialSettings, networkService, bigArrays, threadPool,
xContentRegistry(), new NullDispatcher(), clusterSettings, new SharedGroupFactory(Settings.EMPTY))) {
xContentRegistry(), new NullDispatcher(), clusterSettings, new SharedGroupFactory(Settings.EMPTY),
RestCompatibility.CURRENT_VERSION)) {
transport.start();
TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());
Settings settings = Settings.builder()
.put("http.port", remoteAddress.getPort())
.put("network.host", remoteAddress.getAddress())
.build();
try (Netty4HttpServerTransport otherTransport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool,
xContentRegistry(), new NullDispatcher(), clusterSettings, new SharedGroupFactory(settings))) {
xContentRegistry(), new NullDispatcher(), clusterSettings, new SharedGroupFactory(settings),
RestCompatibility.CURRENT_VERSION)) {
BindHttpException bindHttpException = expectThrows(BindHttpException.class, otherTransport::start);
assertEquals(
"Failed to bind to " + NetworkAddress.format(remoteAddress.address()),
Expand Down Expand Up @@ -258,7 +261,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th

try (Netty4HttpServerTransport transport = new Netty4HttpServerTransport(
settings, networkService, bigArrays, threadPool, xContentRegistry(), dispatcher, clusterSettings,
new SharedGroupFactory(settings))) {
new SharedGroupFactory(settings), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());

Expand Down Expand Up @@ -308,7 +311,7 @@ public void dispatchBadRequest(final RestChannel channel,

try (Netty4HttpServerTransport transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool,
xContentRegistry(), dispatcher, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new SharedGroupFactory(settings))) {
new SharedGroupFactory(settings), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());

Expand Down Expand Up @@ -371,7 +374,7 @@ public void dispatchBadRequest(final RestChannel channel,
NioEventLoopGroup group = new NioEventLoopGroup();
try (Netty4HttpServerTransport transport = new Netty4HttpServerTransport(settings, networkService, bigArrays, threadPool,
xContentRegistry(), dispatcher, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new SharedGroupFactory(settings))) {
new SharedGroupFactory(settings), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.elasticsearch.nio.NioSocketChannel;
import org.elasticsearch.nio.ServerChannelContext;
import org.elasticsearch.nio.SocketChannelContext;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.nio.NioGroupFactory;
import org.elasticsearch.transport.nio.PageAllocator;
Expand Down Expand Up @@ -86,8 +87,9 @@ public class NioHttpServerTransport extends AbstractHttpServerTransport {

public NioHttpServerTransport(Settings settings, NetworkService networkService, BigArrays bigArrays,
PageCacheRecycler pageCacheRecycler, ThreadPool threadPool, NamedXContentRegistry xContentRegistry,
Dispatcher dispatcher, NioGroupFactory nioGroupFactory, ClusterSettings clusterSettings) {
super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher, clusterSettings);
Dispatcher dispatcher, NioGroupFactory nioGroupFactory, ClusterSettings clusterSettings,
RestCompatibility restCompatibleFunction) {
super(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher, clusterSettings, restCompatibleFunction);
this.pageAllocator = new PageAllocator(pageCacheRecycler);
this.nioGroupFactory = nioGroupFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.plugins.NetworkPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.Transport;

Expand Down Expand Up @@ -88,10 +89,11 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports(Settings set
NamedXContentRegistry xContentRegistry,
NetworkService networkService,
HttpServerTransport.Dispatcher dispatcher,
ClusterSettings clusterSettings) {
ClusterSettings clusterSettings,
RestCompatibility restCompatibleFunction) {
return Collections.singletonMap(NIO_HTTP_TRANSPORT_NAME,
() -> new NioHttpServerTransport(settings, networkService, bigArrays, pageCacheRecycler, threadPool, xContentRegistry,
dispatcher, getNioGroupFactory(settings), clusterSettings));
dispatcher, getNioGroupFactory(settings), clusterSettings, restCompatibleFunction));
}

private synchronized NioGroupFactory getNioGroupFactory(Settings settings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.elasticsearch.http.NullDispatcher;
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
import org.elasticsearch.nio.NioSocketChannel;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -162,7 +163,7 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext,
};
try (NioHttpServerTransport transport = new NioHttpServerTransport(settings, networkService, bigArrays, pageRecycler, threadPool,
xContentRegistry(), dispatcher, new NioGroupFactory(settings, logger),
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))) {
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());
try (NioHttpClient client = new NioHttpClient()) {
Expand Down Expand Up @@ -197,7 +198,7 @@ public void testBindUnavailableAddress() {
final Settings initialSettings = createSettings();
try (NioHttpServerTransport transport = new NioHttpServerTransport(initialSettings, networkService, bigArrays, pageRecycler,
threadPool, xContentRegistry(), new NullDispatcher(), new NioGroupFactory(Settings.EMPTY, logger),
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))) {
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), RestCompatibility.CURRENT_VERSION)) {
transport.start();
TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());
Settings settings = Settings.builder()
Expand All @@ -206,7 +207,7 @@ threadPool, xContentRegistry(), new NullDispatcher(), new NioGroupFactory(Settin
.build();
try (NioHttpServerTransport otherTransport = new NioHttpServerTransport(settings, networkService, bigArrays, pageRecycler,
threadPool, xContentRegistry(), new NullDispatcher(), new NioGroupFactory(Settings.EMPTY, logger),
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))) {
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), RestCompatibility.CURRENT_VERSION)) {
BindHttpException bindHttpException = expectThrows(BindHttpException.class, () -> otherTransport.start());
assertEquals(
"Failed to bind to " + NetworkAddress.format(remoteAddress.address()),
Expand Down Expand Up @@ -243,7 +244,7 @@ public void dispatchBadRequest(final RestChannel channel,

try (NioHttpServerTransport transport = new NioHttpServerTransport(settings, networkService, bigArrays, pageRecycler,
threadPool, xContentRegistry(), dispatcher, new NioGroupFactory(settings, logger),
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))) {
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());

Expand Down Expand Up @@ -315,7 +316,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th

try (NioHttpServerTransport transport = new NioHttpServerTransport(settings, networkService, bigArrays, pageRecycler,
threadPool, xContentRegistry(), dispatcher, new NioGroupFactory(settings, logger),
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))) {
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());

Expand Down Expand Up @@ -365,7 +366,7 @@ public void dispatchBadRequest(final RestChannel channel,

try (NioHttpServerTransport transport = new NioHttpServerTransport(settings, networkService, bigArrays, pageRecycler,
threadPool, xContentRegistry(), dispatcher, new NioGroupFactory(settings, logger),
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS))) {
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), RestCompatibility.CURRENT_VERSION)) {
transport.start();
final TransportAddress remoteAddress = randomFrom(transport.boundAddress().boundAddresses());

Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/elasticsearch/Version.java
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return builder.value(toString());
}

public Version previousMajor() {
return Version.fromString(this.major - 1 + ".0.0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you cache this is a class variable ? (i.e. only calculate it once with a null check)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm that would lead to a recursive creation. Maybe we could cache the string to build a previous version major-1+."0.0" or a byte (previousMajor-1)*1000000
then using Version.from* returns a cached instance

}

/*
* We need the declared versions when computing the minimum compatibility version. As computing the declared versions uses reflection it
* is not cheap. Since computing the minimum compatibility version can occur often, we use this holder to compute the declared versions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,6 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr
restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService);
}


public Map<String, ActionHandler<?, ?>> getActions() {
return actions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.elasticsearch.index.shard.PrimaryReplicaSyncer.ResyncTask;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.plugins.NetworkPlugin;
import org.elasticsearch.plugins.RestCompatibility;
import org.elasticsearch.tasks.RawTaskStatus;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -106,6 +107,7 @@ public final class NetworkModule {
/**
* Creates a network module that custom networking classes can be plugged into.
* @param settings The settings for the node
* @param restCompatibleFunction x
*/
public NetworkModule(Settings settings, List<NetworkPlugin> plugins, ThreadPool threadPool,
BigArrays bigArrays,
Expand All @@ -114,11 +116,12 @@ public NetworkModule(Settings settings, List<NetworkPlugin> plugins, ThreadPool
NamedWriteableRegistry namedWriteableRegistry,
NamedXContentRegistry xContentRegistry,
NetworkService networkService, HttpServerTransport.Dispatcher dispatcher,
ClusterSettings clusterSettings) {
ClusterSettings clusterSettings, RestCompatibility restCompatibleFunction) {
this.settings = settings;
for (NetworkPlugin plugin : plugins) {
Map<String, Supplier<HttpServerTransport>> httpTransportFactory = plugin.getHttpTransports(settings, threadPool, bigArrays,
pageCacheRecycler, circuitBreakerService, xContentRegistry, networkService, dispatcher, clusterSettings);
pageCacheRecycler, circuitBreakerService, xContentRegistry, networkService, dispatcher, clusterSettings,
restCompatibleFunction);
for (Map.Entry<String, Supplier<HttpServerTransport>> entry : httpTransportFactory.entrySet()) {
registerHttpTransport(entry.getKey(), entry.getValue());
}
Expand Down
Loading