From 9e8c2133a94dc1eeefe642b8ffc60908f6e108be Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 28 Jan 2020 08:18:11 -0700 Subject: [PATCH 01/18] Introduce system index APIs for Kibana This commit introduces a module for Kibana that exposes REST APIs that will be used by Kibana for access to its system indices. --- modules/kibana/build.gradle | 27 +++++ .../elasticsearch/kibana/KibanaPlugin.java | 113 ++++++++++++++++++ .../kibana/KibanaPluginTests.java | 46 +++++++ .../elasticsearch/action/ActionModule.java | 8 +- .../common/io/stream/StreamInput.java | 17 +++ .../common/io/stream/StreamOutput.java | 16 +++ .../common/settings/ClusterSettings.java | 18 ++- .../common/util/concurrent/ThreadContext.java | 77 +++++++++--- .../java/org/elasticsearch/node/Node.java | 2 +- .../plugins/SystemIndexPlugin.java | 4 +- .../elasticsearch/rest/BaseRestHandler.java | 53 ++++++++ .../elasticsearch/rest/RestController.java | 22 +++- .../transport/InboundMessage.java | 2 +- .../transport/TransportLogger.java | 8 +- .../util/concurrent/ThreadContextTests.java | 6 +- .../authc/AuthenticationServiceTests.java | 2 +- 16 files changed, 395 insertions(+), 26 deletions(-) create mode 100644 modules/kibana/build.gradle create mode 100644 modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java create mode 100644 modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java diff --git a/modules/kibana/build.gradle b/modules/kibana/build.gradle new file mode 100644 index 0000000000000..923b558ca9ff8 --- /dev/null +++ b/modules/kibana/build.gradle @@ -0,0 +1,27 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +esplugin { + description 'Plugin exposing APIs for Kibana system indices' + classname 'org.elasticsearch.kibana.KibanaPlugin' +} + +dependencies { + compileOnly project(path: ':modules:reindex', configuration: 'runtime') +} diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java new file mode 100644 index 0000000000000..dda47fd23ff52 --- /dev/null +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -0,0 +1,113 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.kibana; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.IndexScopedSettings; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Setting.Property; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.settings.SettingsFilter; +import org.elasticsearch.index.reindex.RestDeleteByQueryAction; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.SystemIndexPlugin; +import org.elasticsearch.rest.BaseRestHandler; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestHandler; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.document.RestBulkAction; +import org.elasticsearch.rest.action.document.RestDeleteAction; +import org.elasticsearch.rest.action.document.RestGetAction; +import org.elasticsearch.rest.action.document.RestMultiGetAction; +import org.elasticsearch.rest.action.search.RestSearchAction; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import static java.util.Collections.unmodifiableList; + +public class KibanaPlugin extends Plugin implements SystemIndexPlugin { + + public static final Setting> KIBANA_INDEX_NAMES_SETTING = Setting.listSetting("kibana.system_indices", + unmodifiableList(Arrays.asList(".kibana", ".kibana_task_manager")), Function.identity(), Property.NodeScope); + + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return KIBANA_INDEX_NAMES_SETTING.get(settings).stream() + .map(pattern -> new SystemIndexDescriptor(pattern, "System indices used by kibana")) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + public List getRestHandlers(Settings settings, RestController restController, ClusterSettings clusterSettings, + IndexScopedSettings indexScopedSettings, SettingsFilter settingsFilter, + IndexNameExpressionResolver indexNameExpressionResolver, + Supplier nodesInCluster) { + final List allowedIndexPatterns = KIBANA_INDEX_NAMES_SETTING.get(settings); + return List.of( + new KibanaWrappedRestHandler(new RestGetAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestMultiGetAction(settings), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestSearchAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestBulkAction(settings), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestDeleteAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestDeleteByQueryAction(), allowedIndexPatterns)); + } + + @Override + public List> getSettings() { + return List.of(KIBANA_INDEX_NAMES_SETTING); + } + + static class KibanaWrappedRestHandler extends BaseRestHandler.Wrapper { + + private final List allowedIndexPatterns; + + KibanaWrappedRestHandler(BaseRestHandler delegate, List allowedIndexPatterns) { + super(delegate); + this.allowedIndexPatterns = allowedIndexPatterns; + } + + @Override + public String getName() { + return "kibana_" + super.getName(); + } + + @Override + public List routes() { + return super.routes().stream().map(route -> new Route(route.getMethod(), "/_kibana" + route.getPath())) + .collect(Collectors.toUnmodifiableList()); + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + client.threadPool().getThreadContext().allowSystemIndexAccess(allowedIndexPatterns); + return super.prepareRequest(request, client); + } + } +} diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java new file mode 100644 index 0000000000000..caf612364ff5f --- /dev/null +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java @@ -0,0 +1,46 @@ + +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.kibana; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.test.ESTestCase; + +import java.util.List; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.is; + +public class KibanaPluginTests extends ESTestCase { + + public void testKibanaIndexNames() { + assertThat(new KibanaPlugin().getSettings(), contains(KibanaPlugin.KIBANA_INDEX_NAMES_SETTING)); + assertThat(new KibanaPlugin().getSystemIndexDescriptors(Settings.EMPTY).stream() + .map(SystemIndexDescriptor::getIndexPattern).collect(Collectors.toUnmodifiableList()), + contains(".kibana", ".kibana_task_manager")); + final List names = List.of("." + randomAlphaOfLength(4), "." + randomAlphaOfLength(6)); + final List namesFromDescriptors = new KibanaPlugin().getSystemIndexDescriptors( + Settings.builder().putList(KibanaPlugin.KIBANA_INDEX_NAMES_SETTING.getKey(), names).build() + ).stream().map(SystemIndexDescriptor::getIndexPattern).collect(Collectors.toUnmodifiableList()); + assertThat(namesFromDescriptors, is(names)); + } +} diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 62c265bc17a78..baa73e518b2a7 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -21,6 +21,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Build; import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsAction; @@ -412,9 +413,14 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr indicesAliasesRequestRequestValidators = new RequestValidators<>( actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).collect(Collectors.toList())); - restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService); + final boolean restrictSystemIndices = isSnapshot() && RestController.RESTRICT_SYSTEM_INDICES.get(settings); + restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, restrictSystemIndices); } + // pkg-private for testing + boolean isSnapshot() { + return Build.CURRENT.isSnapshot(); + } public Map> getActions() { return actions; diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java index edc3e98d2f1b1..fbb297468eb04 100644 --- a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java +++ b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java @@ -1145,6 +1145,23 @@ public List readStringList() throws IOException { return readList(StreamInput::readString); } + /** + * Reads an optional list of strings. The list is expected to have been written using + * {@link StreamOutput#writeOptionalStringCollection(Collection)}. If the returned list contains any entries it will be mutable. + * If it is empty it might be immutable. + * + * @return the list of strings + * @throws IOException if an I/O exception occurs reading the list + */ + public List readOptionalStringList() throws IOException { + final boolean isPresent = readBoolean(); + if (isPresent) { + return readList(StreamInput::readString); + } else { + return null; + } + } + /** * Reads a set of objects. If the returned set contains any entries it will be mutable. If it is empty it might be immutable. */ diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java index acdda3732ed6a..88bc2b632ba95 100644 --- a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java +++ b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java @@ -1138,6 +1138,22 @@ public void writeStringCollection(final Collection collection) throws IO writeCollection(collection, StreamOutput::writeString); } + /** + * Writes an optional collection of a strings. The corresponding collection can be read from a stream input using + * {@link StreamInput#readList(Writeable.Reader)}. + * + * @param collection the collection of strings + * @throws IOException if an I/O exception occurs writing the collection + */ + public void writeOptionalStringCollection(final Collection collection) throws IOException { + if (collection != null) { + writeBoolean(true); + writeCollection(collection, StreamOutput::writeString); + } else { + writeBoolean(false); + } + } + /** * Writes a list of {@link NamedWriteable} objects. */ diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 4861cea792144..88397b26d528c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; import org.apache.logging.log4j.LogManager; +import org.elasticsearch.Build; import org.elasticsearch.action.admin.cluster.configuration.TransportAddVotingConfigExclusionsAction; import org.elasticsearch.action.admin.indices.close.TransportCloseIndexAction; import org.elasticsearch.action.search.TransportSearchAction; @@ -97,6 +98,7 @@ import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.rest.BaseRestHandler; +import org.elasticsearch.rest.RestController; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.SearchService; @@ -111,6 +113,7 @@ import org.elasticsearch.watcher.ResourceWatcherService; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.function.Predicate; @@ -180,7 +183,9 @@ public void apply(Settings value, Settings current, Settings previous) { } } - public static Set> BUILT_IN_CLUSTER_SETTINGS = Set.of( + public static final Set> BUILT_IN_CLUSTER_SETTINGS; + static { + final Set> alwaysEnabled = Set.of( AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING, AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING, BalancedShardsAllocator.INDEX_BALANCE_FACTOR_SETTING, @@ -474,6 +479,15 @@ public void apply(Settings value, Settings current, Settings previous) { HandshakingTransportAddressConnector.PROBE_CONNECT_TIMEOUT_SETTING, HandshakingTransportAddressConnector.PROBE_HANDSHAKE_TIMEOUT_SETTING); - static List> BUILT_IN_SETTING_UPGRADERS = Collections.emptyList(); + if (Build.CURRENT.isSnapshot()) { + Set> modifiable = new HashSet<>(alwaysEnabled); + modifiable.add(RestController.RESTRICT_SYSTEM_INDICES); + BUILT_IN_CLUSTER_SETTINGS = Set.copyOf(modifiable); + } else { + BUILT_IN_CLUSTER_SETTINGS = alwaysEnabled; + } + } + + static final List> BUILT_IN_SETTING_UPGRADERS = Collections.emptyList(); } diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java index e4cb14857932f..65fb419eb3040 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java @@ -20,6 +20,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.Version; import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.client.OriginSettingClient; import org.elasticsearch.common.collect.Tuple; @@ -48,6 +49,7 @@ import java.util.stream.Collector; import java.util.stream.Stream; +import static java.util.Collections.emptyList; import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_MAX_WARNING_HEADER_COUNT; import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_MAX_WARNING_HEADER_SIZE; @@ -62,7 +64,7 @@ * Consumers of ThreadContext usually don't need to interact with adding or stashing contexts. Every elasticsearch thread is managed by * a thread pool or executor being responsible for stashing and restoring the threads context. For instance if a network request is * received, all headers are deserialized from the network and directly added as the headers of the threads {@link ThreadContext} - * (see {@link #readHeaders(StreamInput)}. In order to not modify the context that is currently active on this thread the network code + * (see {@link #readFrom(StreamInput)}. In order to not modify the context that is currently active on this thread the network code * uses a try/with pattern to stash it's current context, read headers into a fresh one and once the request is handled or a handler thread * is forked (which in turn inherits the context) it restores the previous context. For instance: *

@@ -230,17 +232,18 @@ public void writeTo(StreamOutput out) throws IOException { } /** - * Reads the headers from the stream into the current context + * Reads the values from the stream into the current context */ - public void readHeaders(StreamInput in) throws IOException { + public void readFrom(StreamInput in) throws IOException { final Tuple, Map>> streamTuple = readHeadersFromStream(in); final Map requestHeaders = streamTuple.v1(); final Map> responseHeaders = streamTuple.v2(); + final List allowedSystemIndices = readAllowedSystemIndices(in); final ThreadContextStruct struct; - if (requestHeaders.isEmpty() && responseHeaders.isEmpty()) { + if (requestHeaders.isEmpty() && responseHeaders.isEmpty() && allowedSystemIndices.isEmpty()) { struct = ThreadContextStruct.EMPTY; } else { - struct = new ThreadContextStruct(requestHeaders, responseHeaders, Collections.emptyMap(), false); + struct = new ThreadContextStruct(requestHeaders, responseHeaders, Collections.emptyMap(), allowedSystemIndices, false, 0L); } threadLocal.set(struct); } @@ -267,6 +270,14 @@ public static Tuple, Map>> readHeadersFr return new Tuple<>(requestHeaders, responseHeaders); } + public static List readAllowedSystemIndices(StreamInput in) throws IOException { + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { // TODO update version on backport + return in.readOptionalStringList(); + } else { + return emptyList(); + } + } + /** * Returns the header for the given key or null if not present */ @@ -410,6 +421,27 @@ public boolean isSystemContext() { return threadLocal.get().isSystemContext; } + /** + * Returns true if a request made within this context can access system indices + */ + public boolean isSystemIndexAccessAllowed() { + return threadLocal.get().allowedSystemIndexPatterns != null; + } + + /** + * Sets the context to disallow access to system indices + */ + public void disallowSystemIndexAccess() { + threadLocal.set(threadLocal.get().setAllowSystemIndices(null)); + } + + /** + * Sets the context to allow access to system indices + */ + public void allowSystemIndexAccess(List patterns) { + threadLocal.set(threadLocal.get().setAllowSystemIndices(patterns)); + } + @FunctionalInterface public interface StoredContext extends AutoCloseable { @Override @@ -441,6 +473,7 @@ private static final class ThreadContextStruct { private final Map requestHeaders; private final Map transientHeaders; private final Map> responseHeaders; + private final List allowedSystemIndexPatterns; private final boolean isSystemContext; //saving current warning headers' size not to recalculate the size with every new warning header private final long warningHeadersSize; @@ -455,29 +488,40 @@ private ThreadContextStruct setSystemContext() { private ThreadContextStruct(Map requestHeaders, Map> responseHeaders, Map transientHeaders, boolean isSystemContext) { - this.requestHeaders = requestHeaders; - this.responseHeaders = responseHeaders; - this.transientHeaders = transientHeaders; - this.isSystemContext = isSystemContext; - this.warningHeadersSize = 0L; + this(requestHeaders, responseHeaders, transientHeaders, emptyList(), isSystemContext, 0L); } private ThreadContextStruct(Map requestHeaders, Map> responseHeaders, Map transientHeaders, boolean isSystemContext, long warningHeadersSize) { + this(requestHeaders, responseHeaders, transientHeaders, emptyList(), isSystemContext, warningHeadersSize); + } + + private ThreadContextStruct(Map requestHeaders, + Map> responseHeaders, + Map transientHeaders, + List allowedSystemIndexPatterns, + boolean isSystemContext, + long warningHeadersSize) { this.requestHeaders = requestHeaders; this.responseHeaders = responseHeaders; this.transientHeaders = transientHeaders; this.isSystemContext = isSystemContext; this.warningHeadersSize = warningHeadersSize; + this.allowedSystemIndexPatterns = allowedSystemIndexPatterns; } /** * This represents the default context and it should only ever be called by {@link #DEFAULT_CONTEXT}. */ private ThreadContextStruct() { - this(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), false); + this(Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap(), emptyList(), false, 0L); + } + + private ThreadContextStruct setAllowSystemIndices(List allowedSystemIndexPatterns) { + return new ThreadContextStruct(requestHeaders, responseHeaders, transientHeaders, allowedSystemIndexPatterns, + isSystemContext, warningHeadersSize); } private ThreadContextStruct putRequest(String key, String value) { @@ -521,7 +565,8 @@ private ThreadContextStruct putResponseHeaders(Map> headers) newResponseHeaders.put(key, entry.getValue()); } } - return new ThreadContextStruct(requestHeaders, newResponseHeaders, transientHeaders, isSystemContext); + return new ThreadContextStruct(requestHeaders, newResponseHeaders, transientHeaders, allowedSystemIndexPatterns, + isSystemContext, 0L); } private ThreadContextStruct putResponse(final String key, final String value, final Function uniqueValue, @@ -571,7 +616,8 @@ private ThreadContextStruct putResponse(final String key, final String value, fi return this; } } - return new ThreadContextStruct(requestHeaders, newResponseHeaders, transientHeaders, isSystemContext, newWarningHeaderSize); + return new ThreadContextStruct(requestHeaders, newResponseHeaders, transientHeaders, allowedSystemIndexPatterns, + isSystemContext, newWarningHeaderSize); } @@ -607,6 +653,9 @@ private void writeTo(StreamOutput out, Map defaultHeaders) throw } out.writeMap(responseHeaders, StreamOutput::writeString, StreamOutput::writeStringCollection); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { // TODO update version on backport + out.writeOptionalStringCollection(allowedSystemIndexPatterns); + } } } @@ -624,7 +673,7 @@ private ContextPreservingRunnable(Runnable in) { @Override public void run() { - try (ThreadContext.StoredContext ignore = stashContext()){ + try (ThreadContext.StoredContext ignore = stashContext()) { ctx.restore(); in.run(); } diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 5566764b2802c..5fded3cc03b0d 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -436,7 +436,7 @@ protected Node( .stream() .collect(Collectors.toUnmodifiableMap( plugin -> plugin.getClass().getSimpleName(), - plugin -> plugin.getSystemIndexDescriptors())); + plugin -> plugin.getSystemIndexDescriptors(settings))); SystemIndexDescriptor.checkForOverlappingPatterns(systemIndexDescriptorMap); final List systemIndexDescriptors = systemIndexDescriptorMap.values().stream() diff --git a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java index a4d7a36f38670..821f141b3eabc 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SystemIndexPlugin.java @@ -19,6 +19,7 @@ package org.elasticsearch.plugins; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndexDescriptor; import java.util.Collection; @@ -33,9 +34,10 @@ public interface SystemIndexPlugin extends ActionPlugin { /** * Returns a {@link Collection} of {@link SystemIndexDescriptor}s that describe this plugin's system indices, including * name, mapping, and settings. + * @param settings The node's settings * @return Descriptions of the system indices managed by this plugin. */ - default Collection getSystemIndexDescriptors() { + default Collection getSystemIndexDescriptors(Settings settings) { return Collections.emptyList(); } } diff --git a/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java index f2fd12bfa79c2..101c3182fbb62 100644 --- a/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/BaseRestHandler.java @@ -183,4 +183,57 @@ protected Set responseParams() { return Collections.emptySet(); } + public static class Wrapper extends BaseRestHandler { + + protected final BaseRestHandler delegate; + + public Wrapper(BaseRestHandler delegate) { + this.delegate = delegate; + } + + @Override + public String getName() { + return delegate.getName(); + } + + @Override + public List routes() { + return delegate.routes(); + } + + @Override + public List deprecatedRoutes() { + return delegate.deprecatedRoutes(); + } + + @Override + public List replacedRoutes() { + return delegate.replacedRoutes(); + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + return delegate.prepareRequest(request, client); + } + + @Override + protected Set responseParams() { + return delegate.responseParams(); + } + + @Override + public boolean canTripCircuitBreaker() { + return delegate.canTripCircuitBreaker(); + } + + @Override + public boolean supportsContentStream() { + return delegate.supportsContentStream(); + } + + @Override + public boolean allowsUnsafeBuffers() { + return delegate.allowsUnsafeBuffers(); + } + } } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index bb015d6632e6b..80938e49aaff1 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -31,6 +31,8 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.path.PathTrie; +import org.elasticsearch.common.settings.Setting; +import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; @@ -64,6 +66,10 @@ public class RestController implements HttpServerTransport.Dispatcher { private static final Logger logger = LogManager.getLogger(RestController.class); private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger); + // TODO once we are ready, this should default to true + public static final Setting RESTRICT_SYSTEM_INDICES = + Setting.boolSetting("rest.restrict_system_indices", false, Property.NodeScope); + private final PathTrie handlers = new PathTrie<>(RestUtils.REST_DECODER); private final UnaryOperator handlerWrapper; @@ -75,9 +81,15 @@ public class RestController implements HttpServerTransport.Dispatcher { /** Rest headers that are copied to internal requests made during a rest request. */ private final Set headersToCopy; private final UsageService usageService; + private final boolean restrictSystemIndices; + + public RestController(Set headersToCopy, UnaryOperator handlerWrapper, + NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) { + this(headersToCopy, handlerWrapper, client, circuitBreakerService, usageService, false); + } public RestController(Set headersToCopy, UnaryOperator handlerWrapper, - NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) { + NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService, boolean restrictSystemIndices) { this.headersToCopy = headersToCopy; this.usageService = usageService; if (handlerWrapper == null) { @@ -86,6 +98,7 @@ public RestController(Set headersToCopy, UnaryOperator Date: Fri, 14 Feb 2020 14:50:05 -0700 Subject: [PATCH 02/18] update method signatures --- .../main/java/org/elasticsearch/tasksplugin/TasksPlugin.java | 3 ++- .../java/org/elasticsearch/tasksplugin/TasksPluginTests.java | 3 ++- .../main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java | 2 +- .../main/java/org/elasticsearch/xpack/logstash/Logstash.java | 2 +- .../main/java/org/elasticsearch/xpack/ml/MachineLearning.java | 2 +- .../main/java/org/elasticsearch/xpack/security/Security.java | 2 +- .../main/java/org/elasticsearch/xpack/transform/Transform.java | 2 +- .../src/main/java/org/elasticsearch/xpack/watcher/Watcher.java | 2 +- 8 files changed, 10 insertions(+), 8 deletions(-) diff --git a/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java b/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java index b7d63991877db..0467b9419c778 100644 --- a/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java +++ b/modules/tasks/src/main/java/org/elasticsearch/tasksplugin/TasksPlugin.java @@ -19,6 +19,7 @@ package org.elasticsearch.tasksplugin; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SystemIndexPlugin; @@ -34,7 +35,7 @@ public class TasksPlugin extends Plugin implements SystemIndexPlugin { @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return Collections.singletonList(new SystemIndexDescriptor(TASK_INDEX, this.getClass().getSimpleName())); } } diff --git a/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java b/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java index 48ec1e06098f3..23b873e377eb3 100644 --- a/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java +++ b/modules/tasks/src/test/java/org/elasticsearch/tasksplugin/TasksPluginTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.tasksplugin; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; @@ -27,6 +28,6 @@ public class TasksPluginTests extends ESTestCase { public void testDummy() { // This is a dummy test case to satisfy the conventions TasksPlugin plugin = new TasksPlugin(); - assertThat(plugin.getSystemIndexDescriptors(), Matchers.hasSize(1)); + assertThat(plugin.getSystemIndexDescriptors(Settings.EMPTY), Matchers.hasSize(1)); } } diff --git a/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java b/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java index 20281a3933bf4..5942203c6233d 100644 --- a/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java +++ b/x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPlugin.java @@ -245,7 +245,7 @@ public List> getSettings() { } @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return Collections.singletonList( new SystemIndexDescriptor(ENRICH_INDEX_PATTERN, "Contains data to support enrich ingest processors.") ); diff --git a/x-pack/plugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/Logstash.java b/x-pack/plugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/Logstash.java index 42c98000f155c..732e425ea2a40 100644 --- a/x-pack/plugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/Logstash.java +++ b/x-pack/plugin/logstash/src/main/java/org/elasticsearch/xpack/logstash/Logstash.java @@ -63,7 +63,7 @@ public UnaryOperator> getIndexTemplateMetaDat } @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return Collections.singletonList(new SystemIndexDescriptor(LOGSTASH_CONCRETE_INDEX_NAME, "Contains data for Logstash Central Management")); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java index 2314ec420482c..0c70e3757b71c 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java @@ -935,7 +935,7 @@ public List getNamedXContent() { } @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return List.of( new SystemIndexDescriptor(MlMetaIndex.INDEX_NAME, "Contains scheduling and anomaly tracking metadata"), new SystemIndexDescriptor(AnomalyDetectorsIndexFields.STATE_INDEX_PATTERN, "Contains ML model state"), diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 8bb3ccc8fbe7b..3791262dc0336 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -1071,7 +1071,7 @@ private synchronized NioGroupFactory getNioGroupFactory(Settings settings) { } @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return List.of( new SystemIndexDescriptor(SECURITY_MAIN_ALIAS, "Contains Security configuration"), new SystemIndexDescriptor(RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_6, "Contains Security configuration"), diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java index 12f82b6427171..d23361d9bb31d 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java @@ -317,7 +317,7 @@ public List getNamedXContent() { } @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return Collections.singletonList( new SystemIndexDescriptor(TransformInternalIndexConstants.INDEX_NAME_PATTERN, "Contains Transform configuration data") ); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 4f1d5f8675a85..0f0712da9679e 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -689,7 +689,7 @@ public void reload(Settings settings) { } @Override - public Collection getSystemIndexDescriptors() { + public Collection getSystemIndexDescriptors(Settings settings) { return List.of( new SystemIndexDescriptor(Watch.INDEX, "Contains Watch definitions"), new SystemIndexDescriptor(TriggeredWatchStoreField.INDEX_NAME, "Used to track current and queued Watch execution") From 8cb8a65cd070403e9d6b35e1421ebdeb49466068 Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 14 Feb 2020 14:57:56 -0700 Subject: [PATCH 03/18] s/compileOnly/compile --- modules/kibana/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/kibana/build.gradle b/modules/kibana/build.gradle index 923b558ca9ff8..f0883c56161f9 100644 --- a/modules/kibana/build.gradle +++ b/modules/kibana/build.gradle @@ -23,5 +23,5 @@ esplugin { } dependencies { - compileOnly project(path: ':modules:reindex', configuration: 'runtime') + compile project(path: ':modules:reindex', configuration: 'runtime') } From 39b6290de81291fd90dd9458183e6ecf385bde8b Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 18 Feb 2020 09:37:49 -0700 Subject: [PATCH 04/18] fix threadcontext serialization --- .../common/io/stream/StreamInput.java | 17 ----------------- .../common/io/stream/StreamOutput.java | 16 ---------------- .../common/util/concurrent/ThreadContext.java | 4 ++-- .../elasticsearch/transport/InboundMessage.java | 1 + 4 files changed, 3 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java index fbb297468eb04..edc3e98d2f1b1 100644 --- a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java +++ b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java @@ -1145,23 +1145,6 @@ public List readStringList() throws IOException { return readList(StreamInput::readString); } - /** - * Reads an optional list of strings. The list is expected to have been written using - * {@link StreamOutput#writeOptionalStringCollection(Collection)}. If the returned list contains any entries it will be mutable. - * If it is empty it might be immutable. - * - * @return the list of strings - * @throws IOException if an I/O exception occurs reading the list - */ - public List readOptionalStringList() throws IOException { - final boolean isPresent = readBoolean(); - if (isPresent) { - return readList(StreamInput::readString); - } else { - return null; - } - } - /** * Reads a set of objects. If the returned set contains any entries it will be mutable. If it is empty it might be immutable. */ diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java index 88bc2b632ba95..acdda3732ed6a 100644 --- a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java +++ b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java @@ -1138,22 +1138,6 @@ public void writeStringCollection(final Collection collection) throws IO writeCollection(collection, StreamOutput::writeString); } - /** - * Writes an optional collection of a strings. The corresponding collection can be read from a stream input using - * {@link StreamInput#readList(Writeable.Reader)}. - * - * @param collection the collection of strings - * @throws IOException if an I/O exception occurs writing the collection - */ - public void writeOptionalStringCollection(final Collection collection) throws IOException { - if (collection != null) { - writeBoolean(true); - writeCollection(collection, StreamOutput::writeString); - } else { - writeBoolean(false); - } - } - /** * Writes a list of {@link NamedWriteable} objects. */ diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java index 65fb419eb3040..2e485e03a291e 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java @@ -272,7 +272,7 @@ public static Tuple, Map>> readHeadersFr public static List readAllowedSystemIndices(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_8_0_0)) { // TODO update version on backport - return in.readOptionalStringList(); + return in.readStringList(); } else { return emptyList(); } @@ -654,7 +654,7 @@ private void writeTo(StreamOutput out, Map defaultHeaders) throw out.writeMap(responseHeaders, StreamOutput::writeString, StreamOutput::writeStringCollection); if (out.getVersion().onOrAfter(Version.V_8_0_0)) { // TODO update version on backport - out.writeOptionalStringCollection(allowedSystemIndexPatterns); + out.writeStringCollection(allowedSystemIndexPatterns); } } } diff --git a/server/src/main/java/org/elasticsearch/transport/InboundMessage.java b/server/src/main/java/org/elasticsearch/transport/InboundMessage.java index 87da6397899c0..94c6f3b49d334 100644 --- a/server/src/main/java/org/elasticsearch/transport/InboundMessage.java +++ b/server/src/main/java/org/elasticsearch/transport/InboundMessage.java @@ -62,6 +62,7 @@ InboundMessage deserialize(BytesReference reference) throws IOException { long requestId = streamInput.readLong(); byte status = streamInput.readByte(); Version remoteVersion = Version.fromId(streamInput.readInt()); + streamInput.setVersion(remoteVersion); final boolean isHandshake = TransportStatus.isHandshake(status); ensureVersionCompatibility(remoteVersion, version, isHandshake); From 758409c253d26d5b5e01bd8360446baa2c2dc9e2 Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 18 Feb 2020 13:45:58 -0700 Subject: [PATCH 05/18] add simple test for client that limits requests/indices --- .../elasticsearch/kibana/KibanaPlugin.java | 79 ++++++++++++++++++- .../kibana/KibanaSystemIndexIT.java | 48 +++++++++++ .../elasticsearch/action/ActionModule.java | 8 +- .../common/io/stream/StreamInput.java | 17 ++++ .../common/io/stream/StreamOutput.java | 16 ++++ .../common/util/concurrent/ThreadContext.java | 4 +- 6 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java index dda47fd23ff52..cd6644f5a9b42 100644 --- a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -19,9 +19,23 @@ package org.elasticsearch.kibana; +import org.apache.lucene.util.automaton.Automaton; +import org.apache.lucene.util.automaton.CharacterRunAutomaton; +import org.apache.lucene.util.automaton.Operations; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequest; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.ActionType; +import org.elasticsearch.action.DocWriteRequest; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.bulk.BulkRequest; +import org.elasticsearch.action.get.MultiGetRequest; +import org.elasticsearch.action.get.MultiGetRequest.Item; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Setting; @@ -107,7 +121,70 @@ public List routes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { client.threadPool().getThreadContext().allowSystemIndexAccess(allowedIndexPatterns); - return super.prepareRequest(request, client); + return super.prepareRequest(request, new IndexLimitingNodeClient(client, allowedIndexPatterns)); + } + } + + static class IndexLimitingNodeClient extends NodeClient { + + private final NodeClient nodeClient; + private final String[] allowedIndexPatterns; + private final Automaton allowedIndexAutomaton; + private final CharacterRunAutomaton automaton; + + IndexLimitingNodeClient(NodeClient nodeClient, List allowedIndexPatterns) { + super(nodeClient.settings(), nodeClient.threadPool()); + this.nodeClient = nodeClient; + this.allowedIndexPatterns = allowedIndexPatterns.toArray(Strings.EMPTY_ARRAY); + this.allowedIndexAutomaton = Regex.simpleMatchToAutomaton(this.allowedIndexPatterns); + this.automaton = new CharacterRunAutomaton(this.allowedIndexAutomaton); + } + + @Override + public + void doExecute(ActionType action, Request request, ActionListener listener) { + final String[] indices; + if (request instanceof BulkRequest) { + indices = ((BulkRequest) request).requests().stream() + .map(DocWriteRequest::index) + .collect(Collectors.toList()) + .toArray(Strings.EMPTY_ARRAY); + } else if (request instanceof MultiGetRequest) { + indices = ((MultiGetRequest) request).getItems().stream() + .map(Item::index) + .collect(Collectors.toList()) + .toArray(Strings.EMPTY_ARRAY); + } else if (request instanceof IndicesRequest) { + indices = ((IndicesRequest) request).indices(); + } else { + listener.onFailure(new IllegalArgumentException("This client cannot be used to make a non indices request")); + return; + } + + if (indices == null || indices.length == 0 || + (indices.length == 1 && (IndexNameExpressionResolver.isAllIndices(List.of(indices)) || indices[0].equals("*")))) { + if (request instanceof IndicesRequest.Replaceable) { + // just replace this with the allowed system index patterns + ((IndicesRequest.Replaceable) request).indices(allowedIndexPatterns); + } else { + listener.onFailure(new IllegalStateException("Unable to replace indices on request " + + request.getClass().getSimpleName())); + return; + } + } else { + for (String index : indices) { + // TODO this will not handle date math, is that OK? Do we need wildcard support? + if (automaton.run(index) == false) { + if (index.contains("*") == false || + Operations.subsetOf(Regex.simpleMatchToAutomaton(index), allowedIndexAutomaton) == false) { + listener.onFailure(new IllegalArgumentException("Index [" + index + "] does not fall within the set " + + Arrays.toString(allowedIndexPatterns) + " allowed by this API")); + return; + } + } + } + } + nodeClient.doExecute(action, request, listener); } } } diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java new file mode 100644 index 0000000000000..ad6653cfb73bf --- /dev/null +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java @@ -0,0 +1,48 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.kibana; + +import org.apache.http.util.EntityUtils; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.test.ESIntegTestCase; + +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; + +public class KibanaSystemIndexIT extends ESIntegTestCase { + + public void testBulkToKibanaIndex() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); + Response response = getRestClient().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request invalidIndexRequest = new Request("POST", "/_kibana/_bulk"); + invalidIndexRequest.setJsonEntity("{ \"index\" : { \"_index\" : \"test\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); + ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + response = e.getResponse(); + assertThat(response.getStatusLine().getStatusCode(), is(400)); + assertThat(EntityUtils.toString(response.getEntity()), containsString("does not fall within the set")); + } +} diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index baa73e518b2a7..7dc53e2c471cf 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -21,7 +21,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.Build; import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.allocation.TransportClusterAllocationExplainAction; import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsAction; @@ -413,15 +412,10 @@ public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpr indicesAliasesRequestRequestValidators = new RequestValidators<>( actionPlugins.stream().flatMap(p -> p.indicesAliasesRequestValidators().stream()).collect(Collectors.toList())); - final boolean restrictSystemIndices = isSnapshot() && RestController.RESTRICT_SYSTEM_INDICES.get(settings); + final boolean restrictSystemIndices = RestController.RESTRICT_SYSTEM_INDICES.get(settings); restController = new RestController(headers, restWrapper, nodeClient, circuitBreakerService, usageService, restrictSystemIndices); } - // pkg-private for testing - boolean isSnapshot() { - return Build.CURRENT.isSnapshot(); - } - public Map> getActions() { return actions; } diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java index edc3e98d2f1b1..fbb297468eb04 100644 --- a/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java +++ b/server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java @@ -1145,6 +1145,23 @@ public List readStringList() throws IOException { return readList(StreamInput::readString); } + /** + * Reads an optional list of strings. The list is expected to have been written using + * {@link StreamOutput#writeOptionalStringCollection(Collection)}. If the returned list contains any entries it will be mutable. + * If it is empty it might be immutable. + * + * @return the list of strings + * @throws IOException if an I/O exception occurs reading the list + */ + public List readOptionalStringList() throws IOException { + final boolean isPresent = readBoolean(); + if (isPresent) { + return readList(StreamInput::readString); + } else { + return null; + } + } + /** * Reads a set of objects. If the returned set contains any entries it will be mutable. If it is empty it might be immutable. */ diff --git a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java index acdda3732ed6a..88bc2b632ba95 100644 --- a/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java +++ b/server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java @@ -1138,6 +1138,22 @@ public void writeStringCollection(final Collection collection) throws IO writeCollection(collection, StreamOutput::writeString); } + /** + * Writes an optional collection of a strings. The corresponding collection can be read from a stream input using + * {@link StreamInput#readList(Writeable.Reader)}. + * + * @param collection the collection of strings + * @throws IOException if an I/O exception occurs writing the collection + */ + public void writeOptionalStringCollection(final Collection collection) throws IOException { + if (collection != null) { + writeBoolean(true); + writeCollection(collection, StreamOutput::writeString); + } else { + writeBoolean(false); + } + } + /** * Writes a list of {@link NamedWriteable} objects. */ diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java index 2e485e03a291e..65fb419eb3040 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java @@ -272,7 +272,7 @@ public static Tuple, Map>> readHeadersFr public static List readAllowedSystemIndices(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_8_0_0)) { // TODO update version on backport - return in.readStringList(); + return in.readOptionalStringList(); } else { return emptyList(); } @@ -654,7 +654,7 @@ private void writeTo(StreamOutput out, Map defaultHeaders) throw out.writeMap(responseHeaders, StreamOutput::writeString, StreamOutput::writeStringCollection); if (out.getVersion().onOrAfter(Version.V_8_0_0)) { // TODO update version on backport - out.writeStringCollection(allowedSystemIndexPatterns); + out.writeOptionalStringCollection(allowedSystemIndexPatterns); } } } From 45776166b848bf3c0a9aef962b9079b47c8b5580 Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 18 Feb 2020 14:14:37 -0700 Subject: [PATCH 06/18] set version on streamgit add .git add .git add . --- .../main/java/org/elasticsearch/transport/TransportLogger.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/transport/TransportLogger.java b/server/src/main/java/org/elasticsearch/transport/TransportLogger.java index b9001c88fbf63..90ff8fe3965f7 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportLogger.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportLogger.java @@ -76,6 +76,7 @@ private static String format(TcpChannel channel, BytesReference message, String final boolean isRequest = TransportStatus.isRequest(status); final String type = isRequest ? "request" : "response"; Version version = Version.fromId(streamInput.readInt()); + streamInput.setVersion(version); sb.append(" [length: ").append(messageLengthWithHeader); sb.append(", request id: ").append(requestId); sb.append(", type: ").append(type); From b0194a8f9cdbd8a238738635f07edb2c4efaa770 Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 19 Feb 2020 13:53:10 -0700 Subject: [PATCH 07/18] add tests for APIs --- modules/kibana/build.gradle | 4 + .../elasticsearch/kibana/KibanaPlugin.java | 26 ++-- .../kibana/KibanaSystemIndexIT.java | 120 ++++++++++++++++++ .../action/RestCancellableNodeClient.java | 52 +++++--- 4 files changed, 172 insertions(+), 30 deletions(-) diff --git a/modules/kibana/build.gradle b/modules/kibana/build.gradle index f0883c56161f9..f9d11e5a6c58b 100644 --- a/modules/kibana/build.gradle +++ b/modules/kibana/build.gradle @@ -25,3 +25,7 @@ esplugin { dependencies { compile project(path: ':modules:reindex', configuration: 'runtime') } + +testClusters.integTest { + module file(project(':modules:reindex').tasks.bundlePlugin.archiveFile) +} diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java index cd6644f5a9b42..d51de575da5a2 100644 --- a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -55,6 +55,7 @@ import org.elasticsearch.rest.action.document.RestGetAction; import org.elasticsearch.rest.action.document.RestMultiGetAction; import org.elasticsearch.rest.action.search.RestSearchAction; +import org.elasticsearch.tasks.Task; import java.io.IOException; import java.util.Arrays; @@ -141,8 +142,9 @@ static class IndexLimitingNodeClient extends NodeClient { } @Override - public - void doExecute(ActionType action, Request request, ActionListener listener) { + public Task executeLocally(ActionType action, + Request request, + ActionListener listener) { final String[] indices; if (request instanceof BulkRequest) { indices = ((BulkRequest) request).requests().stream() @@ -157,8 +159,7 @@ void doExecute(ActionType action, Request request, ActionListener action, Request request, ActionListener action, Request request, ActionListener getRestClient().performRequest(invalidIndexRequest)); + Response invalidResponse = e.getResponse(); + assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); + assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); + } + + public void testMultiGetFromKibanaIndex() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n" + + "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); + request.addParameter("refresh", "true"); + + Response response = getRestClient().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request getRequest = new Request("GET", "/_kibana/_mget"); + getRequest.setJsonEntity("{ \"docs\" : [ { \"_index\" : \".kibana\", \"_id\" : \"1\" }, " + + "{ \"_index\" : \".kibana\", \"_id\" : \"2\" } ] }\n"); + Response getResponse = getRestClient().performRequest(getRequest); + assertThat(getResponse.getStatusLine().getStatusCode(), is(200)); + String responseBody = EntityUtils.toString(getResponse.getEntity()); + assertThat(responseBody, containsString("foo")); + assertThat(responseBody, containsString("bar")); + assertThat(responseBody, containsString("baz")); + assertThat(responseBody, containsString("tag")); + + Request invalidIndexRequest = new Request("GET", "/_kibana/_mget"); + invalidIndexRequest.setJsonEntity("{ \"docs\" : [ { \"_index\" : \"test\", \"_id\" : \"1\" }, " + + "{ \"_index\" : \"test\", \"_id\" : \"2\" } ] }\n"); + ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + Response invalidResponse = e.getResponse(); + assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); + assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); + } + + public void testSearchFromKibanaIndex() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n" + + "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); + request.addParameter("refresh", "true"); + + Response response = getRestClient().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request searchRequest = new Request("GET", "/_kibana/.kibana/_search"); + searchRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); + Response getResponse = getRestClient().performRequest(searchRequest); + assertThat(getResponse.getStatusLine().getStatusCode(), is(200)); + String responseBody = EntityUtils.toString(getResponse.getEntity()); + assertThat(responseBody, containsString("foo")); + assertThat(responseBody, containsString("bar")); + assertThat(responseBody, containsString("baz")); + assertThat(responseBody, containsString("tag")); + + Request invalidIndexRequest = new Request("GET", "/_kibana/test/_search"); + invalidIndexRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); + ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + Response invalidResponse = e.getResponse(); + assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); + assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); + } + + public void testDeleteFromKibanaIndex() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n" + + "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); + request.addParameter("refresh", "true"); + + Response response = getRestClient().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request deleteRequest = new Request("DELETE", "/_kibana/.kibana/_doc/1"); + Response deleteResponse = getRestClient().performRequest(deleteRequest); + assertThat(deleteResponse.getStatusLine().getStatusCode(), is(200)); + + Request invalidIndexRequest = new Request("DELETE", "/_kibana/test/_doc/1"); + ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + Response invalidResponse = e.getResponse(); + assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); + assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); + } + + public void testDeleteByQueryFromKibanaIndex() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n" + + "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); + request.addParameter("refresh", "true"); + + Response response = getRestClient().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request dbqRequest = new Request("POST", "/_kibana/.kibana/_delete_by_query"); + dbqRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); + Response dbqResponse = getRestClient().performRequest(dbqRequest); + assertThat(dbqResponse.getStatusLine().getStatusCode(), is(200)); + + Request invalidIndexRequest = new Request("POST", "/_kibana/.test/_delete_by_query"); + invalidIndexRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); + ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + Response invalidResponse = e.getResponse(); + assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); + assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); + } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/RestCancellableNodeClient.java b/server/src/main/java/org/elasticsearch/rest/action/RestCancellableNodeClient.java index 224c266bd2b18..7d8e71502eb24 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/RestCancellableNodeClient.java +++ b/server/src/main/java/org/elasticsearch/rest/action/RestCancellableNodeClient.java @@ -38,6 +38,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskAction.TASKS_ORIGIN; @@ -85,28 +86,41 @@ static int getNumTasks(HttpChannel channel) { @Override public void doExecute( ActionType action, Request request, ActionListener listener) { - CloseListener closeListener = httpChannels.computeIfAbsent(httpChannel, channel -> new CloseListener()); + final AtomicBoolean created = new AtomicBoolean(false); + CloseListener closeListener = httpChannels.computeIfAbsent(httpChannel, channel -> { + created.set(true); + return new CloseListener(); + }); TaskHolder taskHolder = new TaskHolder(); - Task task = client.executeLocally(action, request, - new ActionListener<>() { - @Override - public void onResponse(Response response) { - try { - closeListener.unregisterTask(taskHolder); - } finally { - listener.onResponse(response); + final Task task; + boolean success = false; + try { + task = client.executeLocally(action, request, + new ActionListener<>() { + @Override + public void onResponse(Response response) { + try { + closeListener.unregisterTask(taskHolder); + } finally { + listener.onResponse(response); + } } - } - - @Override - public void onFailure(Exception e) { - try { - closeListener.unregisterTask(taskHolder); - } finally { - listener.onFailure(e); + + @Override + public void onFailure(Exception e) { + try { + closeListener.unregisterTask(taskHolder); + } finally { + listener.onFailure(e); + } } - } - }); + }); + success = true; + } finally { + if (success == false && created.get()) { + httpChannels.remove(httpChannel); + } + } final TaskId taskId = new TaskId(client.getLocalNodeId(), task.getId()); closeListener.registerTask(taskHolder, taskId); closeListener.maybeRegisterChannel(httpChannel); From 16e5e87685b1084fc88ed06794eeabb914e6bf25 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 20 Feb 2020 10:27:17 -0700 Subject: [PATCH 08/18] rest controller test for system index restriction --- .../elasticsearch/rest/RestController.java | 5 --- .../rest/RestControllerTests.java | 33 +++++++++++++++---- .../rest/RestHttpResponseHeadersTests.java | 2 +- .../indices/RestValidateQueryActionTests.java | 2 +- .../cat/RestCatRecoveryActionTests.java | 2 +- .../action/cat/RestIndicesActionTests.java | 3 +- .../test/rest/RestActionTestCase.java | 2 +- 7 files changed, 32 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 80938e49aaff1..0537b4d40a044 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -83,11 +83,6 @@ public class RestController implements HttpServerTransport.Dispatcher { private final UsageService usageService; private final boolean restrictSystemIndices; - public RestController(Set headersToCopy, UnaryOperator handlerWrapper, - NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService) { - this(headersToCopy, handlerWrapper, client, circuitBreakerService, usageService, false); - } - public RestController(Set headersToCopy, UnaryOperator handlerWrapper, NodeClient client, CircuitBreakerService circuitBreakerService, UsageService usageService, boolean restrictSystemIndices) { this.headersToCopy = headersToCopy; diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 415fe1bf713fa..dfcfd644dde34 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -91,7 +91,7 @@ public void setup() { inFlightRequestsBreaker = circuitBreakerService.getBreaker(CircuitBreaker.IN_FLIGHT_REQUESTS); HttpServerTransport httpServerTransport = new TestHttpServerTransport(); - restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService); + restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService, randomBoolean()); restController.registerHandler(RestRequest.Method.GET, "/", (request, channel, client) -> channel.sendResponse( new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); @@ -106,7 +106,7 @@ public void testApplyRelevantHeaders() throws Exception { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), new RestHeaderDefinition("header.2", true))); - final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService, randomBoolean()); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("true")); restHeaders.put("header.2", Collections.singletonList("true")); @@ -142,7 +142,7 @@ public void testRequestWithDisallowedMultiValuedHeader() { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), new RestHeaderDefinition("header.2", false))); - final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService, randomBoolean()); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("boo")); restHeaders.put("header.2", List.of("foo", "bar")); @@ -156,7 +156,7 @@ public void testRequestWithDisallowedMultiValuedHeaderButSameValues() { final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), new RestHeaderDefinition("header.2", false))); - final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService, randomBoolean()); Map> restHeaders = new HashMap<>(); restHeaders.put("header.1", Collections.singletonList("boo")); restHeaders.put("header.2", List.of("foo", "foo")); @@ -210,7 +210,7 @@ public void testRegisterWithDeprecatedHandler() { } public void testRegisterSecondMethodWithDifferentNamedWildcard() { - final RestController restController = new RestController(null, null, null, circuitBreakerService, usageService); + final RestController restController = new RestController(null, null, null, circuitBreakerService, usageService, randomBoolean()); RestRequest.Method firstMethod = randomFrom(RestRequest.Method.values()); RestRequest.Method secondMethod = @@ -237,7 +237,7 @@ public void testRestHandlerWrapper() throws Exception { h -> { assertSame(handler, h); return (RestRequest request, RestChannel channel, NodeClient client) -> wrapperCalled.set(true); - }, null, circuitBreakerService, usageService); + }, null, circuitBreakerService, usageService, randomBoolean()); restController.registerHandler(RestRequest.Method.GET, "/wrapped", handler); RestRequest request = testRestRequest("/wrapped", "{}", XContentType.JSON); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.BAD_REQUEST); @@ -300,7 +300,7 @@ public void testDispatchRequiresContentTypeForRequestsWithContent() { String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, null); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.NOT_ACCEPTABLE); - restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService); + restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService, randomBoolean()); restController.registerHandler(RestRequest.Method.GET, "/", (r, c, client) -> c.sendResponse( new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); @@ -603,6 +603,25 @@ public HttpRequest releaseAndCopy() { assertThat(channel.getRestResponse().getHeaders().get("Allow"), hasItem(equalTo(RestRequest.Method.GET.toString()))); } + public void testDispatchRestrictSystemIndices() { + restController = new RestController(Collections.emptySet(), null, null, circuitBreakerService, usageService, true); + restController.registerHandler(RestRequest.Method.GET, "/", + (request, channel, client) -> channel.sendResponse( + new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); + restController.registerHandler(RestRequest.Method.GET, "/error", (request, channel, client) -> { + throw new IllegalArgumentException("test error"); + }); + + FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + + assertFalse(channel.getSendResponseCalled()); + ThreadContext context = new ThreadContext(Settings.EMPTY); + assertTrue(context.isSystemIndexAccessAllowed()); + restController.dispatchRequest(fakeRestRequest, channel, context); + assertTrue(channel.getSendResponseCalled()); + assertFalse(context.isSystemIndexAccessAllowed()); + } private static final class TestHttpServerTransport extends AbstractLifecycleComponent implements HttpServerTransport { diff --git a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java b/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java index 6a4a8749397ab..5b4183e72f3e2 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java @@ -89,7 +89,7 @@ public void testUnsupportedMethodResponseHttpHeader() throws Exception { final Settings settings = Settings.EMPTY; UsageService usageService = new UsageService(); RestController restController = new RestController(Collections.emptySet(), - null, null, circuitBreakerService, usageService); + null, null, circuitBreakerService, usageService, randomBoolean()); // A basic RestHandler handles requests to the endpoint RestHandler restHandler = new RestHandler() { diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java index 4813e11e15bfc..7e5a2cf4358db 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java @@ -58,7 +58,7 @@ public class RestValidateQueryActionTests extends AbstractSearchTestCase { private static UsageService usageService = new UsageService(); private static RestController controller = new RestController(emptySet(), null, client, - new NoneCircuitBreakerService(), usageService); + new NoneCircuitBreakerService(), usageService, randomBoolean()); private static RestValidateQueryAction action = new RestValidateQueryAction(); /** diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestCatRecoveryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestCatRecoveryActionTests.java index 258844f36b71c..b62b9d7d0d3fd 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestCatRecoveryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestCatRecoveryActionTests.java @@ -56,7 +56,7 @@ public class RestCatRecoveryActionTests extends ESTestCase { public void testRestRecoveryAction() { final Settings settings = Settings.EMPTY; UsageService usageService = new UsageService(); - final RestController restController = new RestController(Collections.emptySet(), null, null, null, usageService); + final RestController restController = new RestController(Collections.emptySet(), null, null, null, usageService, randomBoolean()); final RestCatRecoveryAction action = new RestCatRecoveryAction(); restController.registerHandler(action); final int totalShards = randomIntBetween(1, 32); diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestIndicesActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestIndicesActionTests.java index aad58e50d69c9..060c2c545875f 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestIndicesActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestIndicesActionTests.java @@ -127,7 +127,8 @@ public void testBuildTable() { } } - final RestController restController = new RestController(Collections.emptySet(), null, null, null, new UsageService()); + final RestController restController = + new RestController(Collections.emptySet(), null, null, null, new UsageService(), randomBoolean()); final RestIndicesAction action = new RestIndicesAction(); restController.registerHandler(action); final Table table = action.buildTable(new FakeRestRequest(), indicesSettings, indicesHealths, indicesStats, indicesMetaDatas); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java index a5d932a3d1a3d..9ac1269a044df 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java @@ -47,7 +47,7 @@ public void setUpController() { controller = new RestController(Collections.emptySet(), null, nodeClient, new NoneCircuitBreakerService(), - new UsageService()); + new UsageService(), randomBoolean()); } /** From d9fa9aef67db86315e4aeda90a8ed808d728e247 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 20 Feb 2020 11:54:19 -0700 Subject: [PATCH 09/18] threadcontext tests --- .../common/util/concurrent/ThreadContext.java | 14 +- .../util/concurrent/ThreadContextTests.java | 127 ++++++++++++++++++ 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java index 65fb419eb3040..6421fec0fc45c 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java @@ -23,6 +23,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.client.OriginSettingClient; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -442,6 +443,15 @@ public void allowSystemIndexAccess(List patterns) { threadLocal.set(threadLocal.get().setAllowSystemIndices(patterns)); } + /** + * Returns the list of allowed system index patterns or {@code null} if none are allowed. An + * empty list indicates that all system indices are allowed to be accessed. + */ + @Nullable + public List allowedSystemIndexPatterns() { + return threadLocal.get().allowedSystemIndexPatterns; + } + @FunctionalInterface public interface StoredContext extends AutoCloseable { @Override @@ -520,8 +530,8 @@ private ThreadContextStruct() { } private ThreadContextStruct setAllowSystemIndices(List allowedSystemIndexPatterns) { - return new ThreadContextStruct(requestHeaders, responseHeaders, transientHeaders, allowedSystemIndexPatterns, - isSystemContext, warningHeadersSize); + final List copy = allowedSystemIndexPatterns == null ? null : List.copyOf(allowedSystemIndexPatterns); + return new ThreadContextStruct(requestHeaders, responseHeaders, transientHeaders, copy, isSystemContext, warningHeadersSize); } private ThreadContextStruct putRequest(String key, String value) { diff --git a/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java b/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java index 9a5137b3efaa2..577b451d8f19e 100644 --- a/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java @@ -29,9 +29,13 @@ import java.util.List; import java.util.Map; import java.util.function.Supplier; + +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; public class ThreadContextTests extends ESTestCase { @@ -254,6 +258,7 @@ public void testSerialize() throws IOException { threadContext.addResponseHeader("Warning", "123456"); } threadContext.addResponseHeader("Warning", "234567"); + threadContext.disallowSystemIndexAccess(); BytesStreamOutput out = new BytesStreamOutput(); threadContext.writeTo(out); @@ -262,6 +267,7 @@ public void testSerialize() throws IOException { assertNull(threadContext.getTransient("ctx.foo")); assertTrue(threadContext.getResponseHeaders().isEmpty()); assertEquals("1", threadContext.getHeader("default")); + assertTrue(threadContext.isSystemIndexAccessAllowed()); threadContext.readFrom(out.bytes().streamInput()); assertEquals("bar", threadContext.getHeader("foo")); @@ -274,10 +280,57 @@ public void testSerialize() throws IOException { assertThat(warnings, hasSize(2)); assertThat(warnings, hasItem(equalTo("123456"))); assertThat(warnings, hasItem(equalTo("234567"))); + + assertFalse(threadContext.isSystemIndexAccessAllowed()); } assertEquals("bar", threadContext.getHeader("foo")); assertEquals(Integer.valueOf(1), threadContext.getTransient("ctx.foo")); assertEquals("1", threadContext.getHeader("default")); + assertFalse(threadContext.isSystemIndexAccessAllowed()); + } + + public void testSerializeWithAllowedSystemIndexPatterns() throws IOException { + Settings build = Settings.builder().put("request.headers.default", "1").build(); + ThreadContext threadContext = new ThreadContext(build); + threadContext.putHeader("foo", "bar"); + threadContext.putTransient("ctx.foo", 1); + threadContext.addResponseHeader("Warning", "123456"); + if (rarely()) { + threadContext.addResponseHeader("Warning", "123456"); + } + threadContext.addResponseHeader("Warning", "234567"); + final List allowed = randomList(1, 8, () -> randomAlphaOfLengthBetween(2, 8)); + threadContext.allowSystemIndexAccess(allowed); + + BytesStreamOutput out = new BytesStreamOutput(); + threadContext.writeTo(out); + try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { + assertNull(threadContext.getHeader("foo")); + assertNull(threadContext.getTransient("ctx.foo")); + assertTrue(threadContext.getResponseHeaders().isEmpty()); + assertEquals("1", threadContext.getHeader("default")); + assertTrue(threadContext.isSystemIndexAccessAllowed()); + + threadContext.readFrom(out.bytes().streamInput()); + assertEquals("bar", threadContext.getHeader("foo")); + assertNull(threadContext.getTransient("ctx.foo")); + + final Map> responseHeaders = threadContext.getResponseHeaders(); + final List warnings = responseHeaders.get("Warning"); + + assertThat(responseHeaders.keySet(), hasSize(1)); + assertThat(warnings, hasSize(2)); + assertThat(warnings, hasItem(equalTo("123456"))); + assertThat(warnings, hasItem(equalTo("234567"))); + + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), equalTo(allowed)); + } + assertEquals("bar", threadContext.getHeader("foo")); + assertEquals(Integer.valueOf(1), threadContext.getTransient("ctx.foo")); + assertEquals("1", threadContext.getHeader("default")); + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), equalTo(allowed)); } public void testSerializeInDifferentContext() throws IOException { @@ -292,11 +345,57 @@ public void testSerializeInDifferentContext() throws IOException { threadContext.addResponseHeader("Warning", "123456"); } threadContext.addResponseHeader("Warning", "234567"); + threadContext.disallowSystemIndexAccess(); + + assertEquals("bar", threadContext.getHeader("foo")); + assertNotNull(threadContext.getTransient("ctx.foo")); + assertEquals("1", threadContext.getHeader("default")); + assertThat(threadContext.getResponseHeaders().keySet(), hasSize(1)); + assertFalse(threadContext.isSystemIndexAccessAllowed()); + threadContext.writeTo(out); + } + { + Settings otherSettings = Settings.builder().put("request.headers.default", "5").build(); + ThreadContext otherThreadContext = new ThreadContext(otherSettings); + otherThreadContext.readFrom(out.bytes().streamInput()); + + assertEquals("bar", otherThreadContext.getHeader("foo")); + assertNull(otherThreadContext.getTransient("ctx.foo")); + assertEquals("1", otherThreadContext.getHeader("default")); + + final Map> responseHeaders = otherThreadContext.getResponseHeaders(); + final List warnings = responseHeaders.get("Warning"); + + assertThat(responseHeaders.keySet(), hasSize(1)); + assertThat(warnings, hasSize(2)); + assertThat(warnings, hasItem(equalTo("123456"))); + assertThat(warnings, hasItem(equalTo("234567"))); + + assertFalse(otherThreadContext.isSystemIndexAccessAllowed()); + } + } + + public void testSerializeInDifferentContextWithAllowedSystemIndices() throws IOException { + final List allowed = randomList(1, 8, () -> randomAlphaOfLengthBetween(2, 8)); + BytesStreamOutput out = new BytesStreamOutput(); + { + Settings build = Settings.builder().put("request.headers.default", "1").build(); + ThreadContext threadContext = new ThreadContext(build); + threadContext.putHeader("foo", "bar"); + threadContext.putTransient("ctx.foo", 1); + threadContext.addResponseHeader("Warning", "123456"); + if (rarely()) { + threadContext.addResponseHeader("Warning", "123456"); + } + threadContext.addResponseHeader("Warning", "234567"); + threadContext.allowSystemIndexAccess(allowed); assertEquals("bar", threadContext.getHeader("foo")); assertNotNull(threadContext.getTransient("ctx.foo")); assertEquals("1", threadContext.getHeader("default")); assertThat(threadContext.getResponseHeaders().keySet(), hasSize(1)); + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), equalTo(allowed)); threadContext.writeTo(out); } { @@ -315,6 +414,9 @@ public void testSerializeInDifferentContext() throws IOException { assertThat(warnings, hasSize(2)); assertThat(warnings, hasItem(equalTo("123456"))); assertThat(warnings, hasItem(equalTo("234567"))); + + assertTrue(otherThreadContext.isSystemIndexAccessAllowed()); + assertThat(otherThreadContext.allowedSystemIndexPatterns(), equalTo(allowed)); } } @@ -613,6 +715,31 @@ public void testPutHeaders() { assertEquals("value for key [foo] already present", e.getMessage()); } + public void testSystemIndexAccessAllowed() { + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), empty()); + try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { + assertTrue(threadContext.isSystemIndexAccessAllowed()); + threadContext.disallowSystemIndexAccess(); + assertFalse(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), nullValue()); + } + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), empty()); + + try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { + threadContext.disallowSystemIndexAccess(); + final List allowed = randomList(1, 8, () -> randomAlphaOfLengthBetween(2, 8)); + threadContext.allowSystemIndexAccess(allowed); + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), not(sameInstance(allowed))); + assertThat(threadContext.allowedSystemIndexPatterns(), equalTo(allowed)); + } + assertTrue(threadContext.isSystemIndexAccessAllowed()); + assertThat(threadContext.allowedSystemIndexPatterns(), empty()); + } + /** * Sometimes wraps a Runnable in an AbstractRunnable. */ From 2dabec85fd4a289cfc5e4227e0d95cc815929a8d Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 20 Feb 2020 13:26:29 -0700 Subject: [PATCH 10/18] no randomBoolean in static value --- .../rest/action/admin/indices/RestValidateQueryActionTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java index 7e5a2cf4358db..f7c060a48f527 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java @@ -58,7 +58,7 @@ public class RestValidateQueryActionTests extends AbstractSearchTestCase { private static UsageService usageService = new UsageService(); private static RestController controller = new RestController(emptySet(), null, client, - new NoneCircuitBreakerService(), usageService, randomBoolean()); + new NoneCircuitBreakerService(), usageService, false); private static RestValidateQueryAction action = new RestValidateQueryAction(); /** From 2a749968b156b57cc1258f4272f37415f03c9325 Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 26 Feb 2020 13:33:31 -0700 Subject: [PATCH 11/18] add APIs listed in Kibana issue --- .../elasticsearch/kibana/KibanaPlugin.java | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java index d51de575da5a2..7317156387215 100644 --- a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -50,11 +50,21 @@ import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.action.admin.indices.RestCreateIndexAction; +import org.elasticsearch.rest.action.admin.indices.RestGetAliasesAction; +import org.elasticsearch.rest.action.admin.indices.RestGetIndicesAction; +import org.elasticsearch.rest.action.admin.indices.RestIndexPutAliasAction; +import org.elasticsearch.rest.action.admin.indices.RestRefreshAction; +import org.elasticsearch.rest.action.admin.indices.RestUpdateSettingsAction; import org.elasticsearch.rest.action.document.RestBulkAction; import org.elasticsearch.rest.action.document.RestDeleteAction; import org.elasticsearch.rest.action.document.RestGetAction; +import org.elasticsearch.rest.action.document.RestIndexAction; import org.elasticsearch.rest.action.document.RestMultiGetAction; +import org.elasticsearch.rest.action.document.RestUpdateAction; +import org.elasticsearch.rest.action.search.RestClearScrollAction; import org.elasticsearch.rest.action.search.RestSearchAction; +import org.elasticsearch.rest.action.search.RestSearchScrollAction; import org.elasticsearch.tasks.Task; import java.io.IOException; @@ -70,7 +80,7 @@ public class KibanaPlugin extends Plugin implements SystemIndexPlugin { public static final Setting> KIBANA_INDEX_NAMES_SETTING = Setting.listSetting("kibana.system_indices", - unmodifiableList(Arrays.asList(".kibana", ".kibana_task_manager")), Function.identity(), Property.NodeScope); + unmodifiableList(Arrays.asList(".kibana", ".kibana_task_manager", ".reporting")), Function.identity(), Property.NodeScope); @Override public Collection getSystemIndexDescriptors(Settings settings) { @@ -86,12 +96,32 @@ public List getRestHandlers(Settings settings, RestController restC Supplier nodesInCluster) { final List allowedIndexPatterns = KIBANA_INDEX_NAMES_SETTING.get(settings); return List.of( + // Based on https://github.com/elastic/kibana/issues/49764 + // apis needed to perform migrations... ideally these will go away + new KibanaWrappedRestHandler(new RestCreateIndexAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestGetAliasesAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestIndexPutAliasAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestRefreshAction(), allowedIndexPatterns), + + // apis needed to access saved objects new KibanaWrappedRestHandler(new RestGetAction(), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestMultiGetAction(settings), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestSearchAction(), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestBulkAction(settings), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestDeleteAction(), allowedIndexPatterns), - new KibanaWrappedRestHandler(new RestDeleteByQueryAction(), allowedIndexPatterns)); + new KibanaWrappedRestHandler(new RestDeleteByQueryAction(), allowedIndexPatterns), + + // api used for testing + new KibanaWrappedRestHandler(new RestUpdateSettingsAction(), allowedIndexPatterns), + + // apis used specifically by reporting + new KibanaWrappedRestHandler(new RestGetIndicesAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestIndexAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestUpdateAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestSearchScrollAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new RestClearScrollAction(), allowedIndexPatterns) + ); + } @Override From 2a8b63747fe4a06df2ac63192c9c5c8931370dd5 Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 26 Feb 2020 13:53:46 -0700 Subject: [PATCH 12/18] add reporting --- .../test/java/org/elasticsearch/kibana/KibanaPluginTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java index caf612364ff5f..bf6d472b1e77a 100644 --- a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java @@ -36,7 +36,7 @@ public void testKibanaIndexNames() { assertThat(new KibanaPlugin().getSettings(), contains(KibanaPlugin.KIBANA_INDEX_NAMES_SETTING)); assertThat(new KibanaPlugin().getSystemIndexDescriptors(Settings.EMPTY).stream() .map(SystemIndexDescriptor::getIndexPattern).collect(Collectors.toUnmodifiableList()), - contains(".kibana", ".kibana_task_manager")); + contains(".kibana", ".kibana_task_manager", ".reporting")); final List names = List.of("." + randomAlphaOfLength(4), "." + randomAlphaOfLength(6)); final List namesFromDescriptors = new KibanaPlugin().getSystemIndexDescriptors( Settings.builder().putList(KibanaPlugin.KIBANA_INDEX_NAMES_SETTING.getKey(), names).build() From 863fe79e00d9482d0a6fb7c1ad0f02d891a1de22 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 27 Feb 2020 08:27:46 -0700 Subject: [PATCH 13/18] address description --- .../src/main/java/org/elasticsearch/kibana/KibanaPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java index 7317156387215..07e902b3b221a 100644 --- a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -85,7 +85,7 @@ public class KibanaPlugin extends Plugin implements SystemIndexPlugin { @Override public Collection getSystemIndexDescriptors(Settings settings) { return KIBANA_INDEX_NAMES_SETTING.get(settings).stream() - .map(pattern -> new SystemIndexDescriptor(pattern, "System indices used by kibana")) + .map(pattern -> new SystemIndexDescriptor(pattern, "System index used by kibana")) .collect(Collectors.toUnmodifiableList()); } From b1c53f7020533cc4ae9f09fae434dab979e71754 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 27 Feb 2020 08:41:03 -0700 Subject: [PATCH 14/18] ESRestTestCase --- .../kibana/KibanaSystemIndexIT.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java index 12340f9e27e1c..e0abc96498ef7 100644 --- a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java @@ -23,24 +23,24 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; -import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.rest.ESRestTestCase; import java.io.IOException; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; -public class KibanaSystemIndexIT extends ESIntegTestCase { +public class KibanaSystemIndexIT extends ESRestTestCase { public void testBulkToKibanaIndex() throws IOException { Request request = new Request("POST", "/_kibana/_bulk"); request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); - Response response = getRestClient().performRequest(request); + Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); Request invalidIndexRequest = new Request("POST", "/_kibana/_bulk"); invalidIndexRequest.setJsonEntity("{ \"index\" : { \"_index\" : \"test\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); response = e.getResponse(); assertThat(response.getStatusLine().getStatusCode(), is(400)); assertThat(EntityUtils.toString(response.getEntity()), containsString("does not fall within the set")); @@ -51,18 +51,18 @@ public void testGetFromKibanaIndex() throws IOException { request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); request.addParameter("refresh", "true"); - Response response = getRestClient().performRequest(request); + Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); Request getRequest = new Request("GET", "/_kibana/.kibana/_doc/1"); - Response getResponse = getRestClient().performRequest(getRequest); + Response getResponse = client().performRequest(getRequest); assertThat(getResponse.getStatusLine().getStatusCode(), is(200)); String responseBody = EntityUtils.toString(getResponse.getEntity()); assertThat(responseBody, containsString("foo")); assertThat(responseBody, containsString("bar")); Request invalidIndexRequest = new Request("GET", "/_kibana/test/_doc/1"); - ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); Response invalidResponse = e.getResponse(); assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); @@ -74,13 +74,13 @@ public void testMultiGetFromKibanaIndex() throws IOException { "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); request.addParameter("refresh", "true"); - Response response = getRestClient().performRequest(request); + Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); Request getRequest = new Request("GET", "/_kibana/_mget"); getRequest.setJsonEntity("{ \"docs\" : [ { \"_index\" : \".kibana\", \"_id\" : \"1\" }, " + "{ \"_index\" : \".kibana\", \"_id\" : \"2\" } ] }\n"); - Response getResponse = getRestClient().performRequest(getRequest); + Response getResponse = client().performRequest(getRequest); assertThat(getResponse.getStatusLine().getStatusCode(), is(200)); String responseBody = EntityUtils.toString(getResponse.getEntity()); assertThat(responseBody, containsString("foo")); @@ -91,7 +91,7 @@ public void testMultiGetFromKibanaIndex() throws IOException { Request invalidIndexRequest = new Request("GET", "/_kibana/_mget"); invalidIndexRequest.setJsonEntity("{ \"docs\" : [ { \"_index\" : \"test\", \"_id\" : \"1\" }, " + "{ \"_index\" : \"test\", \"_id\" : \"2\" } ] }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); Response invalidResponse = e.getResponse(); assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); @@ -103,12 +103,12 @@ public void testSearchFromKibanaIndex() throws IOException { "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); request.addParameter("refresh", "true"); - Response response = getRestClient().performRequest(request); + Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); Request searchRequest = new Request("GET", "/_kibana/.kibana/_search"); searchRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); - Response getResponse = getRestClient().performRequest(searchRequest); + Response getResponse = client().performRequest(searchRequest); assertThat(getResponse.getStatusLine().getStatusCode(), is(200)); String responseBody = EntityUtils.toString(getResponse.getEntity()); assertThat(responseBody, containsString("foo")); @@ -118,7 +118,7 @@ public void testSearchFromKibanaIndex() throws IOException { Request invalidIndexRequest = new Request("GET", "/_kibana/test/_search"); invalidIndexRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); Response invalidResponse = e.getResponse(); assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); @@ -130,15 +130,15 @@ public void testDeleteFromKibanaIndex() throws IOException { "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); request.addParameter("refresh", "true"); - Response response = getRestClient().performRequest(request); + Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); Request deleteRequest = new Request("DELETE", "/_kibana/.kibana/_doc/1"); - Response deleteResponse = getRestClient().performRequest(deleteRequest); + Response deleteResponse = client().performRequest(deleteRequest); assertThat(deleteResponse.getStatusLine().getStatusCode(), is(200)); Request invalidIndexRequest = new Request("DELETE", "/_kibana/test/_doc/1"); - ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); Response invalidResponse = e.getResponse(); assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); @@ -150,17 +150,17 @@ public void testDeleteByQueryFromKibanaIndex() throws IOException { "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n"); request.addParameter("refresh", "true"); - Response response = getRestClient().performRequest(request); + Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); Request dbqRequest = new Request("POST", "/_kibana/.kibana/_delete_by_query"); dbqRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); - Response dbqResponse = getRestClient().performRequest(dbqRequest); + Response dbqResponse = client().performRequest(dbqRequest); assertThat(dbqResponse.getStatusLine().getStatusCode(), is(200)); Request invalidIndexRequest = new Request("POST", "/_kibana/.test/_delete_by_query"); invalidIndexRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> getRestClient().performRequest(invalidIndexRequest)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); Response invalidResponse = e.getResponse(); assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); From eaac89d6175859bd594c4e9de142ecdd387733b1 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 27 Feb 2020 09:22:11 -0700 Subject: [PATCH 15/18] cleanup stream version setting --- .../PublicationTransportHandler.java | 6 ++-- .../common/compress/DeflateCompressor.java | 9 ++++-- .../org/elasticsearch/common/io/Streams.java | 11 +++++++ .../transport/InboundMessage.java | 32 +++++++++++-------- .../transport/OutboundMessage.java | 3 +- .../transport/TransportLogger.java | 7 ++-- .../common/compress/DeflateCompressTests.java | 7 ++++ 7 files changed, 53 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/PublicationTransportHandler.java b/server/src/main/java/org/elasticsearch/cluster/coordination/PublicationTransportHandler.java index 4b451df2814fe..a4b2f509b2ed8 100644 --- a/server/src/main/java/org/elasticsearch/cluster/coordination/PublicationTransportHandler.java +++ b/server/src/main/java/org/elasticsearch/cluster/coordination/PublicationTransportHandler.java @@ -319,8 +319,8 @@ private void sendClusterStateDiff(ClusterState clusterState, public static BytesReference serializeFullClusterState(ClusterState clusterState, Version nodeVersion) throws IOException { final BytesStreamOutput bStream = new BytesStreamOutput(); + bStream.setVersion(nodeVersion); try (StreamOutput stream = CompressorFactory.COMPRESSOR.streamOutput(bStream)) { - stream.setVersion(nodeVersion); stream.writeBoolean(true); clusterState.writeTo(stream); } @@ -329,8 +329,8 @@ public static BytesReference serializeFullClusterState(ClusterState clusterState public static BytesReference serializeDiffClusterState(Diff diff, Version nodeVersion) throws IOException { final BytesStreamOutput bStream = new BytesStreamOutput(); + bStream.setVersion(nodeVersion); try (StreamOutput stream = CompressorFactory.COMPRESSOR.streamOutput(bStream)) { - stream.setVersion(nodeVersion); stream.writeBoolean(false); diff.writeTo(stream); } @@ -340,12 +340,12 @@ public static BytesReference serializeDiffClusterState(Diff diff, Version nodeVe private PublishWithJoinResponse handleIncomingPublishRequest(BytesTransportRequest request) throws IOException { final Compressor compressor = CompressorFactory.compressor(request.bytes()); StreamInput in = request.bytes().streamInput(); + in.setVersion(request.version()); try { if (compressor != null) { in = compressor.streamInput(in); } in = new NamedWriteableAwareStreamInput(in, namedWriteableRegistry); - in.setVersion(request.version()); // If true we received full cluster state - otherwise diffs if (in.readBoolean()) { final ClusterState incomingState; diff --git a/server/src/main/java/org/elasticsearch/common/compress/DeflateCompressor.java b/server/src/main/java/org/elasticsearch/common/compress/DeflateCompressor.java index 794a8db4960c6..646e6c6138230 100644 --- a/server/src/main/java/org/elasticsearch/common/compress/DeflateCompressor.java +++ b/server/src/main/java/org/elasticsearch/common/compress/DeflateCompressor.java @@ -85,7 +85,7 @@ public StreamInput streamInput(StreamInput in) throws IOException { final Inflater inflater = new Inflater(nowrap); InputStream decompressedIn = new InflaterInputStream(in, inflater, BUFFER_SIZE); decompressedIn = new BufferedInputStream(decompressedIn, BUFFER_SIZE); - return new InputStreamStreamInput(decompressedIn) { + final InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(decompressedIn) { final AtomicBoolean closed = new AtomicBoolean(false); public void close() throws IOException { @@ -99,6 +99,9 @@ public void close() throws IOException { } } }; + + inputStreamStreamInput.setVersion(in.getVersion()); + return inputStreamStreamInput; } @Override @@ -109,7 +112,7 @@ public StreamOutput streamOutput(StreamOutput out) throws IOException { final boolean syncFlush = true; DeflaterOutputStream deflaterOutputStream = new DeflaterOutputStream(out, deflater, BUFFER_SIZE, syncFlush); OutputStream compressedOut = new BufferedOutputStream(deflaterOutputStream, BUFFER_SIZE); - return new OutputStreamStreamOutput(compressedOut) { + final OutputStreamStreamOutput outputStreamStreamOutput = new OutputStreamStreamOutput(compressedOut) { final AtomicBoolean closed = new AtomicBoolean(false); public void close() throws IOException { @@ -123,5 +126,7 @@ public void close() throws IOException { } } }; + outputStreamStreamOutput.setVersion(out.getVersion()); + return outputStreamStreamOutput; } } diff --git a/server/src/main/java/org/elasticsearch/common/io/Streams.java b/server/src/main/java/org/elasticsearch/common/io/Streams.java index 222f94e65ef6a..3747c4d895a20 100644 --- a/server/src/main/java/org/elasticsearch/common/io/Streams.java +++ b/server/src/main/java/org/elasticsearch/common/io/Streams.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.io; +import org.elasticsearch.Version; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStream; import org.elasticsearch.common.io.stream.BytesStreamOutput; @@ -296,5 +297,15 @@ public void reset() throws IOException { public BytesReference bytes() { return delegate.bytes(); } + + @Override + public Version getVersion() { + return delegate.getVersion(); + } + + @Override + public void setVersion(Version version) { + delegate.setVersion(version); + } } } diff --git a/server/src/main/java/org/elasticsearch/transport/InboundMessage.java b/server/src/main/java/org/elasticsearch/transport/InboundMessage.java index 94c6f3b49d334..14db21bc86ec4 100644 --- a/server/src/main/java/org/elasticsearch/transport/InboundMessage.java +++ b/server/src/main/java/org/elasticsearch/transport/InboundMessage.java @@ -61,7 +61,7 @@ InboundMessage deserialize(BytesReference reference) throws IOException { try (ThreadContext.StoredContext existing = threadContext.stashContext()) { long requestId = streamInput.readLong(); byte status = streamInput.readByte(); - Version remoteVersion = Version.fromId(streamInput.readInt()); + final Version remoteVersion = Version.fromId(streamInput.readInt()); streamInput.setVersion(remoteVersion); final boolean isHandshake = TransportStatus.isHandshake(status); ensureVersionCompatibility(remoteVersion, version, isHandshake); @@ -70,7 +70,8 @@ InboundMessage deserialize(BytesReference reference) throws IOException { // Consume the variable header size streamInput.readInt(); } else { - streamInput = decompressingStream(status, remoteVersion, streamInput); + streamInput = decompressingStream(status, streamInput); + assertRemoteVersion(streamInput, remoteVersion); } threadContext.readFrom(streamInput); @@ -84,15 +85,18 @@ InboundMessage deserialize(BytesReference reference) throws IOException { final String action = streamInput.readString(); if (remoteVersion.onOrAfter(TcpHeader.VERSION_WITH_HEADER_SIZE)) { - streamInput = decompressingStream(status, remoteVersion, streamInput); + streamInput = decompressingStream(status, streamInput); + assertRemoteVersion(streamInput, remoteVersion); } - streamInput = namedWriteableStream(streamInput, remoteVersion); + streamInput = namedWriteableStream(streamInput); message = new Request(threadContext, remoteVersion, status, requestId, action, streamInput); } else { if (remoteVersion.onOrAfter(TcpHeader.VERSION_WITH_HEADER_SIZE)) { - streamInput = decompressingStream(status, remoteVersion, streamInput); + streamInput = decompressingStream(status, streamInput); + assertRemoteVersion(streamInput, remoteVersion); } - streamInput = namedWriteableStream(streamInput, remoteVersion); + streamInput = namedWriteableStream(streamInput); + assertRemoteVersion(streamInput, remoteVersion); message = new Response(threadContext, remoteVersion, status, requestId, streamInput); } success = true; @@ -104,12 +108,10 @@ InboundMessage deserialize(BytesReference reference) throws IOException { } } - static StreamInput decompressingStream(byte status, Version remoteVersion, StreamInput streamInput) throws IOException { + static StreamInput decompressingStream(byte status, StreamInput streamInput) throws IOException { if (TransportStatus.isCompress(status) && streamInput.available() > 0) { try { - StreamInput decompressor = CompressorFactory.COMPRESSOR.streamInput(streamInput); - decompressor.setVersion(remoteVersion); - return decompressor; + return CompressorFactory.COMPRESSOR.streamInput(streamInput); } catch (IllegalArgumentException e) { throw new IllegalStateException("stream marked as compressed, but is missing deflate header"); } @@ -118,10 +120,12 @@ static StreamInput decompressingStream(byte status, Version remoteVersion, Strea } } - private StreamInput namedWriteableStream(StreamInput delegate, Version remoteVersion) { - NamedWriteableAwareStreamInput streamInput = new NamedWriteableAwareStreamInput(delegate, namedWriteableRegistry); - streamInput.setVersion(remoteVersion); - return streamInput; + private StreamInput namedWriteableStream(StreamInput delegate) { + return new NamedWriteableAwareStreamInput(delegate, namedWriteableRegistry); + } + + static void assertRemoteVersion(StreamInput in, Version version) { + assert version.equals(in.getVersion()) : "Stream version [" + in.getVersion() + "] does not match version [" + version + "]"; } } diff --git a/server/src/main/java/org/elasticsearch/transport/OutboundMessage.java b/server/src/main/java/org/elasticsearch/transport/OutboundMessage.java index 16ba7a15a539c..bae4cb0d982d5 100644 --- a/server/src/main/java/org/elasticsearch/transport/OutboundMessage.java +++ b/server/src/main/java/org/elasticsearch/transport/OutboundMessage.java @@ -55,7 +55,8 @@ BytesReference serialize(BytesStreamOutput bytesStream) throws IOException { } try (CompressibleBytesOutputStream stream = new CompressibleBytesOutputStream(bytesStream, TransportStatus.isCompress(status))) { - stream.setVersion(version); + assert stream.getVersion().equals(version) : + "Stream version [" + stream.getVersion() + "] does not match version [" + version + "]"; if (variableHeaderLength == -1) { writeVariableHeader(stream); } diff --git a/server/src/main/java/org/elasticsearch/transport/TransportLogger.java b/server/src/main/java/org/elasticsearch/transport/TransportLogger.java index 90ff8fe3965f7..7543ecfa304d4 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportLogger.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportLogger.java @@ -28,6 +28,8 @@ import java.io.IOException; +import static org.elasticsearch.transport.InboundMessage.Reader.assertRemoteVersion; + public final class TransportLogger { private static final Logger logger = LogManager.getLogger(TransportLogger.class); @@ -75,7 +77,7 @@ private static String format(TcpChannel channel, BytesReference message, String final byte status = streamInput.readByte(); final boolean isRequest = TransportStatus.isRequest(status); final String type = isRequest ? "request" : "response"; - Version version = Version.fromId(streamInput.readInt()); + final Version version = Version.fromId(streamInput.readInt()); streamInput.setVersion(version); sb.append(" [length: ").append(messageLengthWithHeader); sb.append(", request id: ").append(requestId); @@ -85,7 +87,8 @@ private static String format(TcpChannel channel, BytesReference message, String if (version.onOrAfter(TcpHeader.VERSION_WITH_HEADER_SIZE)) { sb.append(", header size: ").append(streamInput.readInt()).append('B'); } else { - streamInput = InboundMessage.Reader.decompressingStream(status, version, streamInput); + streamInput = InboundMessage.Reader.decompressingStream(status, streamInput); + assertRemoteVersion(streamInput, version); } // TODO (jaymode) Need a better way to deal with this. In one aspect, diff --git a/server/src/test/java/org/elasticsearch/common/compress/DeflateCompressTests.java b/server/src/test/java/org/elasticsearch/common/compress/DeflateCompressTests.java index 33d11aa23d890..c02b4e460d28d 100644 --- a/server/src/test/java/org/elasticsearch/common/compress/DeflateCompressTests.java +++ b/server/src/test/java/org/elasticsearch/common/compress/DeflateCompressTests.java @@ -21,11 +21,13 @@ import org.apache.lucene.util.LineFileDocs; import org.apache.lucene.util.TestUtil; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.ByteBufferStreamInput; import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -388,9 +390,12 @@ private void doTest(byte bytes[]) throws IOException { StreamInput rawIn = new ByteBufferStreamInput(bb); Compressor c = compressor; + final Version version = VersionUtils.randomVersion(random()); ByteArrayOutputStream bos = new ByteArrayOutputStream(); OutputStreamStreamOutput rawOs = new OutputStreamStreamOutput(bos); + rawOs.setVersion(version); StreamOutput os = c.streamOutput(rawOs); + assertEquals(version, os.getVersion()); Random r = random(); int bufferSize = r.nextBoolean() ? 65535 : TestUtil.nextInt(random(), 1, 70000); @@ -410,7 +415,9 @@ private void doTest(byte bytes[]) throws IOException { byte compressed[] = bos.toByteArray(); ByteBuffer bb2 = ByteBuffer.wrap(compressed); StreamInput compressedIn = new ByteBufferStreamInput(bb2); + compressedIn.setVersion(version); StreamInput in = c.streamInput(compressedIn); + assertEquals(version, in.getVersion()); // randomize constants again bufferSize = r.nextBoolean() ? 65535 : TestUtil.nextInt(random(), 1, 70000); From 43d413408beae535989cf4a3d7cb2231f399ba43 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 27 Feb 2020 09:54:04 -0700 Subject: [PATCH 16/18] Fix version on CompressibleBytesOutputStream --- .../transport/CompressibleBytesOutputStream.java | 11 +++++++++++ .../CompressibleBytesOutputStreamTests.java | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/transport/CompressibleBytesOutputStream.java b/server/src/main/java/org/elasticsearch/transport/CompressibleBytesOutputStream.java index 4116f88b14224..7cb60a4326aa5 100644 --- a/server/src/main/java/org/elasticsearch/transport/CompressibleBytesOutputStream.java +++ b/server/src/main/java/org/elasticsearch/transport/CompressibleBytesOutputStream.java @@ -19,6 +19,7 @@ package org.elasticsearch.transport; +import org.elasticsearch.Version; import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressorFactory; @@ -102,4 +103,14 @@ public void close() throws IOException { public void reset() throws IOException { throw new UnsupportedOperationException(); } + + @Override + public Version getVersion() { + return stream.getVersion(); + } + + @Override + public void setVersion(Version version) { + stream.setVersion(version); + } } diff --git a/server/src/test/java/org/elasticsearch/transport/CompressibleBytesOutputStreamTests.java b/server/src/test/java/org/elasticsearch/transport/CompressibleBytesOutputStreamTests.java index aeb92dac73479..90f251bbc5610 100644 --- a/server/src/test/java/org/elasticsearch/transport/CompressibleBytesOutputStreamTests.java +++ b/server/src/test/java/org/elasticsearch/transport/CompressibleBytesOutputStreamTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import java.io.EOFException; import java.io.IOException; @@ -33,7 +34,11 @@ public class CompressibleBytesOutputStreamTests extends ESTestCase { public void testStreamWithoutCompression() throws IOException { BytesStream bStream = new ZeroOutOnCloseStream(); + if (randomBoolean()) { + bStream.setVersion(VersionUtils.randomVersion(random())); + } CompressibleBytesOutputStream stream = new CompressibleBytesOutputStream(bStream, false); + assertEquals(bStream.getVersion(), stream.getVersion()); byte[] expectedBytes = randomBytes(randomInt(30)); stream.write(expectedBytes); @@ -61,7 +66,11 @@ public void testStreamWithoutCompression() throws IOException { public void testStreamWithCompression() throws IOException { BytesStream bStream = new ZeroOutOnCloseStream(); + if (randomBoolean()) { + bStream.setVersion(VersionUtils.randomVersion(random())); + } CompressibleBytesOutputStream stream = new CompressibleBytesOutputStream(bStream, true); + assertEquals(bStream.getVersion(), stream.getVersion()); byte[] expectedBytes = randomBytes(randomInt(30)); stream.write(expectedBytes); @@ -88,7 +97,11 @@ public void testStreamWithCompression() throws IOException { public void testCompressionWithCallingMaterializeFails() throws IOException { BytesStream bStream = new ZeroOutOnCloseStream(); + if (randomBoolean()) { + bStream.setVersion(VersionUtils.randomVersion(random())); + } CompressibleBytesOutputStream stream = new CompressibleBytesOutputStream(bStream, true); + assertEquals(bStream.getVersion(), stream.getVersion()); byte[] expectedBytes = randomBytes(between(1, 30)); stream.write(expectedBytes); From 57781291bb9e7af851de2015ba593de768d403ed Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 27 Feb 2020 13:21:59 -0700 Subject: [PATCH 17/18] remove limiting of indices --- .../elasticsearch/kibana/KibanaPlugin.java | 91 +------------------ .../kibana/KibanaPluginTests.java | 2 +- .../kibana/KibanaSystemIndexIT.java | 42 --------- 3 files changed, 5 insertions(+), 130 deletions(-) diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java index 07e902b3b221a..1c7f8c70ec2eb 100644 --- a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -19,23 +19,9 @@ package org.elasticsearch.kibana; -import org.apache.lucene.util.automaton.Automaton; -import org.apache.lucene.util.automaton.CharacterRunAutomaton; -import org.apache.lucene.util.automaton.Operations; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.ActionRequest; -import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.ActionType; -import org.elasticsearch.action.DocWriteRequest; -import org.elasticsearch.action.IndicesRequest; -import org.elasticsearch.action.bulk.BulkRequest; -import org.elasticsearch.action.get.MultiGetRequest; -import org.elasticsearch.action.get.MultiGetRequest.Item; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.common.Strings; -import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Setting; @@ -65,22 +51,18 @@ import org.elasticsearch.rest.action.search.RestClearScrollAction; import org.elasticsearch.rest.action.search.RestSearchAction; import org.elasticsearch.rest.action.search.RestSearchScrollAction; -import org.elasticsearch.tasks.Task; import java.io.IOException; -import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; -import static java.util.Collections.unmodifiableList; - public class KibanaPlugin extends Plugin implements SystemIndexPlugin { public static final Setting> KIBANA_INDEX_NAMES_SETTING = Setting.listSetting("kibana.system_indices", - unmodifiableList(Arrays.asList(".kibana", ".kibana_task_manager", ".reporting")), Function.identity(), Property.NodeScope); + List.of(".kibana*", ".reporting"), Function.identity(), Property.NodeScope); @Override public Collection getSystemIndexDescriptors(Settings settings) { @@ -94,7 +76,8 @@ public List getRestHandlers(Settings settings, RestController restC IndexScopedSettings indexScopedSettings, SettingsFilter settingsFilter, IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster) { - final List allowedIndexPatterns = KIBANA_INDEX_NAMES_SETTING.get(settings); + // TODO need to figure out what subset of system indices Kibana should have access to via these APIs + final List allowedIndexPatterns = List.of(); return List.of( // Based on https://github.com/elastic/kibana/issues/49764 // apis needed to perform migrations... ideally these will go away @@ -152,73 +135,7 @@ public List routes() { @Override protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { client.threadPool().getThreadContext().allowSystemIndexAccess(allowedIndexPatterns); - return super.prepareRequest(request, new IndexLimitingNodeClient(client, allowedIndexPatterns)); - } - } - - static class IndexLimitingNodeClient extends NodeClient { - - private final NodeClient nodeClient; - private final String[] allowedIndexPatterns; - private final Automaton allowedIndexAutomaton; - private final CharacterRunAutomaton automaton; - - IndexLimitingNodeClient(NodeClient nodeClient, List allowedIndexPatterns) { - super(nodeClient.settings(), nodeClient.threadPool()); - this.nodeClient = nodeClient; - this.allowedIndexPatterns = allowedIndexPatterns.toArray(Strings.EMPTY_ARRAY); - this.allowedIndexAutomaton = Regex.simpleMatchToAutomaton(this.allowedIndexPatterns); - this.automaton = new CharacterRunAutomaton(this.allowedIndexAutomaton); - } - - @Override - public Task executeLocally(ActionType action, - Request request, - ActionListener listener) { - final String[] indices; - if (request instanceof BulkRequest) { - indices = ((BulkRequest) request).requests().stream() - .map(DocWriteRequest::index) - .collect(Collectors.toList()) - .toArray(Strings.EMPTY_ARRAY); - } else if (request instanceof MultiGetRequest) { - indices = ((MultiGetRequest) request).getItems().stream() - .map(Item::index) - .collect(Collectors.toList()) - .toArray(Strings.EMPTY_ARRAY); - } else if (request instanceof IndicesRequest) { - indices = ((IndicesRequest) request).indices(); - } else { - throw new IllegalArgumentException("This client cannot be used to make a non indices request"); - } - - if (indices == null || indices.length == 0 || - (indices.length == 1 && (IndexNameExpressionResolver.isAllIndices(List.of(indices)) || indices[0].equals("*")))) { - if (request instanceof IndicesRequest.Replaceable) { - // just replace this with the allowed system index patterns - ((IndicesRequest.Replaceable) request).indices(allowedIndexPatterns); - } else { - throw new IllegalStateException("Unable to replace indices on request " + - request.getClass().getSimpleName()); - } - } else { - for (String index : indices) { - // TODO this will not handle date math, is that OK? Do we need wildcard support? - if (automaton.run(index) == false) { - if (index.contains("*") == false || - Operations.subsetOf(Regex.simpleMatchToAutomaton(index), allowedIndexAutomaton) == false) { - throw new IllegalArgumentException("Index [" + index + "] does not fall within the set " - + Arrays.toString(allowedIndexPatterns) + " allowed by this API"); - } - } - } - } - return nodeClient.executeLocally(action, request, listener); - } - - @Override - public String getLocalNodeId() { - return nodeClient.getLocalNodeId(); + return super.prepareRequest(request, client); } } } diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java index bf6d472b1e77a..5094dd7178bcb 100644 --- a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaPluginTests.java @@ -36,7 +36,7 @@ public void testKibanaIndexNames() { assertThat(new KibanaPlugin().getSettings(), contains(KibanaPlugin.KIBANA_INDEX_NAMES_SETTING)); assertThat(new KibanaPlugin().getSystemIndexDescriptors(Settings.EMPTY).stream() .map(SystemIndexDescriptor::getIndexPattern).collect(Collectors.toUnmodifiableList()), - contains(".kibana", ".kibana_task_manager", ".reporting")); + contains(".kibana*", ".reporting")); final List names = List.of("." + randomAlphaOfLength(4), "." + randomAlphaOfLength(6)); final List namesFromDescriptors = new KibanaPlugin().getSystemIndexDescriptors( Settings.builder().putList(KibanaPlugin.KIBANA_INDEX_NAMES_SETTING.getKey(), names).build() diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java index e0abc96498ef7..73dbf71bea51f 100644 --- a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java @@ -22,7 +22,6 @@ import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; -import org.elasticsearch.client.ResponseException; import org.elasticsearch.test.rest.ESRestTestCase; import java.io.IOException; @@ -37,13 +36,6 @@ public void testBulkToKibanaIndex() throws IOException { request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); Response response = client().performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); - - Request invalidIndexRequest = new Request("POST", "/_kibana/_bulk"); - invalidIndexRequest.setJsonEntity("{ \"index\" : { \"_index\" : \"test\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); - response = e.getResponse(); - assertThat(response.getStatusLine().getStatusCode(), is(400)); - assertThat(EntityUtils.toString(response.getEntity()), containsString("does not fall within the set")); } public void testGetFromKibanaIndex() throws IOException { @@ -60,12 +52,6 @@ public void testGetFromKibanaIndex() throws IOException { String responseBody = EntityUtils.toString(getResponse.getEntity()); assertThat(responseBody, containsString("foo")); assertThat(responseBody, containsString("bar")); - - Request invalidIndexRequest = new Request("GET", "/_kibana/test/_doc/1"); - ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); - Response invalidResponse = e.getResponse(); - assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); - assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); } public void testMultiGetFromKibanaIndex() throws IOException { @@ -87,14 +73,6 @@ public void testMultiGetFromKibanaIndex() throws IOException { assertThat(responseBody, containsString("bar")); assertThat(responseBody, containsString("baz")); assertThat(responseBody, containsString("tag")); - - Request invalidIndexRequest = new Request("GET", "/_kibana/_mget"); - invalidIndexRequest.setJsonEntity("{ \"docs\" : [ { \"_index\" : \"test\", \"_id\" : \"1\" }, " + - "{ \"_index\" : \"test\", \"_id\" : \"2\" } ] }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); - Response invalidResponse = e.getResponse(); - assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); - assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); } public void testSearchFromKibanaIndex() throws IOException { @@ -115,13 +93,6 @@ public void testSearchFromKibanaIndex() throws IOException { assertThat(responseBody, containsString("bar")); assertThat(responseBody, containsString("baz")); assertThat(responseBody, containsString("tag")); - - Request invalidIndexRequest = new Request("GET", "/_kibana/test/_search"); - invalidIndexRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); - Response invalidResponse = e.getResponse(); - assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); - assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); } public void testDeleteFromKibanaIndex() throws IOException { @@ -136,12 +107,6 @@ public void testDeleteFromKibanaIndex() throws IOException { Request deleteRequest = new Request("DELETE", "/_kibana/.kibana/_doc/1"); Response deleteResponse = client().performRequest(deleteRequest); assertThat(deleteResponse.getStatusLine().getStatusCode(), is(200)); - - Request invalidIndexRequest = new Request("DELETE", "/_kibana/test/_doc/1"); - ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); - Response invalidResponse = e.getResponse(); - assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); - assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); } public void testDeleteByQueryFromKibanaIndex() throws IOException { @@ -157,12 +122,5 @@ public void testDeleteByQueryFromKibanaIndex() throws IOException { dbqRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); Response dbqResponse = client().performRequest(dbqRequest); assertThat(dbqResponse.getStatusLine().getStatusCode(), is(200)); - - Request invalidIndexRequest = new Request("POST", "/_kibana/.test/_delete_by_query"); - invalidIndexRequest.setJsonEntity("{ \"query\" : { \"match_all\" : {} } }\n"); - ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(invalidIndexRequest)); - Response invalidResponse = e.getResponse(); - assertThat(invalidResponse.getStatusLine().getStatusCode(), is(400)); - assertThat(EntityUtils.toString(invalidResponse.getEntity()), containsString("does not fall within the set")); } } From 52b609da993681225bd484ad25d01ab4d327ef58 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 27 Feb 2020 14:30:38 -0700 Subject: [PATCH 18/18] add tests for rest of apis --- .../elasticsearch/kibana/KibanaPlugin.java | 4 + .../kibana/KibanaSystemIndexIT.java | 123 ++++++++++++++++++ .../elasticsearch/action/ActionModule.java | 7 +- .../java/org/elasticsearch/node/Node.java | 2 +- .../rest/action/document/RestIndexAction.java | 11 +- .../action/ActionModuleTests.java | 6 +- .../action/document/RestIndexActionTests.java | 7 +- 7 files changed, 140 insertions(+), 20 deletions(-) diff --git a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java index 1c7f8c70ec2eb..f917c477493cc 100644 --- a/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java +++ b/modules/kibana/src/main/java/org/elasticsearch/kibana/KibanaPlugin.java @@ -46,6 +46,8 @@ import org.elasticsearch.rest.action.document.RestDeleteAction; import org.elasticsearch.rest.action.document.RestGetAction; import org.elasticsearch.rest.action.document.RestIndexAction; +import org.elasticsearch.rest.action.document.RestIndexAction.AutoIdHandler; +import org.elasticsearch.rest.action.document.RestIndexAction.CreateHandler; import org.elasticsearch.rest.action.document.RestMultiGetAction; import org.elasticsearch.rest.action.document.RestUpdateAction; import org.elasticsearch.rest.action.search.RestClearScrollAction; @@ -100,6 +102,8 @@ public List getRestHandlers(Settings settings, RestController restC // apis used specifically by reporting new KibanaWrappedRestHandler(new RestGetIndicesAction(), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestIndexAction(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new CreateHandler(), allowedIndexPatterns), + new KibanaWrappedRestHandler(new AutoIdHandler(nodesInCluster), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestUpdateAction(), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestSearchScrollAction(), allowedIndexPatterns), new KibanaWrappedRestHandler(new RestClearScrollAction(), allowedIndexPatterns) diff --git a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java index 73dbf71bea51f..f3901112e839f 100644 --- a/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java +++ b/modules/kibana/src/test/java/org/elasticsearch/kibana/KibanaSystemIndexIT.java @@ -22,15 +22,39 @@ import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.rest.ESRestTestCase; import java.io.IOException; +import java.util.Map; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; public class KibanaSystemIndexIT extends ESRestTestCase { + public void testCreateIndex() throws IOException { + Request request = new Request("PUT", "/_kibana/.kibana-1"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + } + + public void testAliases() throws IOException { + Request request = new Request("PUT", "/_kibana/.kibana-1"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + request = new Request("PUT", "/_kibana/.kibana-1/_alias/.kibana"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + request = new Request("GET", "/_kibana/_aliases"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + assertThat(EntityUtils.toString(response.getEntity()), containsString(".kibana")); + } + public void testBulkToKibanaIndex() throws IOException { Request request = new Request("POST", "/_kibana/_bulk"); request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); @@ -38,6 +62,24 @@ public void testBulkToKibanaIndex() throws IOException { assertThat(response.getStatusLine().getStatusCode(), is(200)); } + public void testRefresh() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + request = new Request("GET", "/_kibana/.kibana/_refresh"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request getRequest = new Request("GET", "/_kibana/.kibana/_doc/1"); + Response getResponse = client().performRequest(getRequest); + assertThat(getResponse.getStatusLine().getStatusCode(), is(200)); + String responseBody = EntityUtils.toString(getResponse.getEntity()); + assertThat(responseBody, containsString("foo")); + assertThat(responseBody, containsString("bar")); + } + public void testGetFromKibanaIndex() throws IOException { Request request = new Request("POST", "/_kibana/_bulk"); request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n"); @@ -123,4 +165,85 @@ public void testDeleteByQueryFromKibanaIndex() throws IOException { Response dbqResponse = client().performRequest(dbqRequest); assertThat(dbqResponse.getStatusLine().getStatusCode(), is(200)); } + + public void testUpdateIndexSettings() throws IOException { + Request request = new Request("PUT", "/_kibana/.kibana-1"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + request = new Request("PUT", "/_kibana/.kibana-1/_settings"); + request.setJsonEntity("{ \"index.blocks.read_only\" : false }"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + } + + public void testGetIndex() throws IOException { + Request request = new Request("PUT", "/_kibana/.kibana-1"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + request = new Request("GET", "/_kibana/.kibana-1"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + assertThat(EntityUtils.toString(response.getEntity()), containsString(".kibana-1")); + } + + public void testIndexingAndUpdatingDocs() throws IOException { + Request request = new Request("PUT", "/_kibana/.kibana-1/_doc/1"); + request.setJsonEntity("{ \"foo\" : \"bar\" }"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(201)); + + request = new Request("PUT", "/_kibana/.kibana-1/_create/2"); + request.setJsonEntity("{ \"foo\" : \"bar\" }"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(201)); + + request = new Request("POST", "/_kibana/.kibana-1/_doc"); + request.setJsonEntity("{ \"foo\" : \"bar\" }"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(201)); + + request = new Request("GET", "/_kibana/.kibana-1/_refresh"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + request = new Request("POST", "/_kibana/.kibana-1/_update/1"); + request.setJsonEntity("{ \"doc\" : { \"foo\" : \"baz\" } }"); + response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + } + + public void testScrollingDocs() throws IOException { + Request request = new Request("POST", "/_kibana/_bulk"); + request.setJsonEntity("{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"1\" } }\n{ \"foo\" : \"bar\" }\n" + + "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"2\" } }\n{ \"baz\" : \"tag\" }\n" + + "{ \"index\" : { \"_index\" : \".kibana\", \"_id\" : \"3\" } }\n{ \"baz\" : \"tag\" }\n"); + request.addParameter("refresh", "true"); + Response response = client().performRequest(request); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + + Request searchRequest = new Request("GET", "/_kibana/.kibana/_search"); + searchRequest.setJsonEntity("{ \"size\" : 1,\n\"query\" : { \"match_all\" : {} } }\n"); + searchRequest.addParameter("scroll", "1m"); + response = client().performRequest(searchRequest); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + Map map = XContentHelper.convertToMap(JsonXContent.jsonXContent, EntityUtils.toString(response.getEntity()), false); + assertNotNull(map.get("_scroll_id")); + String scrollId = (String) map.get("_scroll_id"); + + Request scrollRequest = new Request("POST", "/_kibana/_search/scroll"); + scrollRequest.addParameter("scroll_id", scrollId); + scrollRequest.addParameter("scroll", "1m"); + response = client().performRequest(scrollRequest); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + map = XContentHelper.convertToMap(JsonXContent.jsonXContent, EntityUtils.toString(response.getEntity()), false); + assertNotNull(map.get("_scroll_id")); + scrollId = (String) map.get("_scroll_id"); + + Request clearScrollRequest = new Request("DELETE", "/_kibana/_search/scroll"); + clearScrollRequest.addParameter("scroll_id", scrollId); + response = client().performRequest(clearScrollRequest); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + } } diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 7dc53e2c471cf..affbb7a41dd31 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -205,7 +205,6 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.NamedRegistry; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.TypeLiteral; @@ -375,19 +374,17 @@ public class ActionModule extends AbstractModule { private final RestController restController; private final RequestValidators mappingRequestValidators; private final RequestValidators indicesAliasesRequestRequestValidators; - private final ClusterService clusterService; public ActionModule(Settings settings, IndexNameExpressionResolver indexNameExpressionResolver, IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter, ThreadPool threadPool, List actionPlugins, NodeClient nodeClient, - CircuitBreakerService circuitBreakerService, UsageService usageService, ClusterService clusterService) { + CircuitBreakerService circuitBreakerService, UsageService usageService) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; this.indexScopedSettings = indexScopedSettings; this.clusterSettings = clusterSettings; this.settingsFilter = settingsFilter; this.actionPlugins = actionPlugins; - this.clusterService = clusterService; actions = setupActions(actionPlugins); actionFilters = setupActionFilters(actionPlugins); autoCreateIndex = new AutoCreateIndex(settings, clusterSettings, indexNameExpressionResolver); @@ -639,7 +636,7 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestIndexAction()); registerHandler.accept(new CreateHandler()); - registerHandler.accept(new AutoIdHandler(clusterService)); + registerHandler.accept(new AutoIdHandler(nodesInCluster)); registerHandler.accept(new RestGetAction()); registerHandler.accept(new RestGetSourceAction()); registerHandler.accept(new RestMultiGetAction(settings)); diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index d0fe4f80ef69a..a6fa9e9faa492 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -476,7 +476,7 @@ protected Node(final Environment initialEnvironment, ActionModule actionModule = new ActionModule(settings, clusterModule.getIndexNameExpressionResolver(), settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(), - threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, clusterService); + threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService); modules.add(actionModule); final RestController restController = actionModule.getRestController(); diff --git a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java index 4a86ca118b176..f155b09aafb42 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/document/RestIndexAction.java @@ -23,7 +23,7 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.index.VersionType; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; @@ -33,6 +33,7 @@ import java.io.IOException; import java.util.List; import java.util.Locale; +import java.util.function.Supplier; import static org.elasticsearch.rest.RestRequest.Method.POST; import static org.elasticsearch.rest.RestRequest.Method.PUT; @@ -81,10 +82,10 @@ void validateOpType(String opType) { public static final class AutoIdHandler extends RestIndexAction { - private final ClusterService clusterService; + private final Supplier nodesInCluster; - public AutoIdHandler(ClusterService clusterService) { - this.clusterService = clusterService; + public AutoIdHandler(Supplier nodesInCluster) { + this.nodesInCluster = nodesInCluster; } @Override @@ -100,7 +101,7 @@ public List routes() { @Override public RestChannelConsumer prepareRequest(RestRequest request, final NodeClient client) throws IOException { assert request.params().get("id") == null : "non-null id: " + request.params().get("id"); - if (request.params().get("op_type") == null && clusterService.state().nodes().getMinNodeVersion().onOrAfter(Version.V_7_5_0)) { + if (request.params().get("op_type") == null && nodesInCluster.get().getMinNodeVersion().onOrAfter(Version.V_7_5_0)) { // default to op_type create request.params().put("op_type", "create"); } diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index 114abe0b2dcb7..61118d31b3a2a 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -109,7 +109,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() { UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), null, emptyList(), null, - null, usageService, null); + null, usageService); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail Exception e = expectThrows(IllegalArgumentException.class, () -> @@ -141,7 +141,7 @@ public List getRestHandlers(Settings settings, RestController restC UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, - singletonList(dupsMainAction), null, null, usageService, null); + singletonList(dupsMainAction), null, null, usageService); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET")); } finally { @@ -175,7 +175,7 @@ public List getRestHandlers(Settings settings, RestController restC UsageService usageService = new UsageService(); ActionModule actionModule = new ActionModule(settings.getSettings(), new IndexNameExpressionResolver(), settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool, - singletonList(registersFakeHandler), null, null, usageService, null); + singletonList(registersFakeHandler), null, null, usageService); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail Exception e = expectThrows(IllegalArgumentException.class, () -> diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java index 84bc5f3bef340..bb549b64c724f 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestIndexActionTests.java @@ -27,7 +27,6 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.rest.RestRequest; @@ -43,9 +42,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class RestIndexActionTests extends RestActionTestCase { @@ -53,11 +50,9 @@ public class RestIndexActionTests extends RestActionTestCase { @Before public void setUpAction() { - ClusterService clusterService = mock(ClusterService.class); - when(clusterService.state()).thenAnswer(invocationOnMock -> clusterStateSupplier.get()); controller().registerHandler(new RestIndexAction()); controller().registerHandler(new CreateHandler()); - controller().registerHandler(new AutoIdHandler(clusterService)); + controller().registerHandler(new AutoIdHandler(() -> clusterStateSupplier.get().nodes())); } public void testCreateOpTypeValidation() {