Skip to content

Commit

Permalink
System index auto-creation should not be disabled by user settings (#…
Browse files Browse the repository at this point in the history
…62984) (#63147)

* Add System Indices check to AutoCreateIndex

By default, Elasticsearch auto-creates indices when a document is
submitted to a non-existent index. There is a setting that allows users
to disable this behavior. However, this setting should not apply to
system indices, so that Elasticsearch modules and plugins are able to
use auto-create behavior whether or not it is exposed to users.

This commit constructs the AutoCreateIndex object with a reference to
the SystemIndices object so that we bypass the check for the user-facing
autocreate setting when it's a system index that is being autocreated.

We also modify the logic in TransportBulkAction to make sure that if a
system index is included in a bulk request, we don't skip the
autocreation step.
  • Loading branch information
williamrandolph committed Oct 1, 2020
1 parent fc13b72 commit 6899ce6
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.indices.SystemIndices;
import org.elasticsearch.test.ESTestCase;

import java.util.HashMap;

import static java.util.Collections.emptyMap;
import static org.hamcrest.Matchers.containsString;

Expand All @@ -60,7 +63,8 @@ public class ReindexSourceTargetValidationTests extends ESTestCase {
.put(index("source2", "source_multi"), true)).build();
private static final IndexNameExpressionResolver INDEX_NAME_EXPRESSION_RESOLVER = new IndexNameExpressionResolver();
private static final AutoCreateIndex AUTO_CREATE_INDEX = new AutoCreateIndex(Settings.EMPTY,
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), INDEX_NAME_EXPRESSION_RESOLVER);
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), INDEX_NAME_EXPRESSION_RESOLVER,
new SystemIndices(new HashMap<>()));

private final BytesReference query = new BytesArray("{ \"foo\" : \"bar\" }");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsFilter;
import org.elasticsearch.index.seqno.RetentionLeaseActions;
import org.elasticsearch.indices.SystemIndices;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.persistent.CompletionPersistentTaskAction;
import org.elasticsearch.persistent.RemovePersistentTaskAction;
Expand Down Expand Up @@ -427,7 +428,7 @@ public class ActionModule extends AbstractModule {
public ActionModule(boolean transportClient, Settings settings, IndexNameExpressionResolver indexNameExpressionResolver,
IndexScopedSettings indexScopedSettings, ClusterSettings clusterSettings, SettingsFilter settingsFilter,
ThreadPool threadPool, List<ActionPlugin> actionPlugins, NodeClient nodeClient,
CircuitBreakerService circuitBreakerService, UsageService usageService) {
CircuitBreakerService circuitBreakerService, UsageService usageService, SystemIndices systemIndices) {
this.transportClient = transportClient;
this.settings = settings;
this.indexNameExpressionResolver = indexNameExpressionResolver;
Expand All @@ -438,7 +439,9 @@ public ActionModule(boolean transportClient, Settings settings, IndexNameExpress
this.threadPool = threadPool;
actions = setupActions(actionPlugins);
actionFilters = setupActionFilters(actionPlugins);
autoCreateIndex = transportClient ? null : new AutoCreateIndex(settings, clusterSettings, indexNameExpressionResolver);
autoCreateIndex = transportClient
? null
: new AutoCreateIndex(settings, clusterSettings, indexNameExpressionResolver, systemIndices);
destructiveOperations = new DestructiveOperations(settings, clusterSettings);
Set<RestHeaderDefinition> headers = Stream.concat(
actionPlugins.stream().flatMap(p -> p.getRestHeaders().stream()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,9 @@ protected void doInternalExecute(Task task, BulkRequest bulkRequest, String exec
return;
}

if (needToCheck()) {
final boolean includesSystem = includesSystem(bulkRequest, clusterService.state().metadata().getIndicesLookup(), systemIndices);

if (includesSystem || needToCheck()) {
// Attempt to create all the indices that we're going to need during the bulk before we start.
// Step 1: collect all the indices in the request
final Map<String, Boolean> indices = bulkRequest.requests.stream()
Expand Down Expand Up @@ -352,15 +354,20 @@ static void prohibitCustomRoutingOnDataStream(DocWriteRequest<?> writeRequest, M
}

boolean isOnlySystem(BulkRequest request, SortedMap<String, IndexAbstraction> indicesLookup, SystemIndices systemIndices) {
final boolean onlySystem = request.getIndices().stream().allMatch(indexName -> {
final IndexAbstraction abstraction = indicesLookup.get(indexName);
if (abstraction != null) {
return abstraction.isSystem();
} else {
return systemIndices.isSystemIndex(indexName);
}
});
return onlySystem;
return request.getIndices().stream().allMatch(indexName -> isSystemIndex(indicesLookup, systemIndices, indexName));
}

boolean includesSystem(BulkRequest request, SortedMap<String, IndexAbstraction> indicesLookup, SystemIndices systemIndices) {
return request.getIndices().stream().anyMatch(indexName -> isSystemIndex(indicesLookup, systemIndices, indexName));
}

private boolean isSystemIndex(SortedMap<String, IndexAbstraction> indicesLookup, SystemIndices systemIndices, String indexName) {
final IndexAbstraction abstraction = indicesLookup.get(indexName);
if (abstraction != null) {
return abstraction.isSystem();
} else {
return systemIndices.isSystemIndex(indexName);
}
}

boolean needToCheck() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.indices.SystemIndices;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -46,11 +47,16 @@ public final class AutoCreateIndex {

private final boolean dynamicMappingDisabled;
private final IndexNameExpressionResolver resolver;
private final SystemIndices systemIndices;
private volatile AutoCreate autoCreate;

public AutoCreateIndex(Settings settings, ClusterSettings clusterSettings, IndexNameExpressionResolver resolver) {
public AutoCreateIndex(Settings settings,
ClusterSettings clusterSettings,
IndexNameExpressionResolver resolver,
SystemIndices systemIndices) {
this.resolver = resolver;
dynamicMappingDisabled = !MapperService.INDEX_MAPPER_DYNAMIC_SETTING.get(settings);
this.systemIndices = systemIndices;
this.autoCreate = AUTO_CREATE_INDEX_SETTING.get(settings);
clusterSettings.addSettingsUpdateConsumer(AUTO_CREATE_INDEX_SETTING, this::setAutoCreate);
}
Expand All @@ -70,6 +76,12 @@ public boolean shouldAutoCreate(String index, ClusterState state) {
if (resolver.hasIndexAbstraction(index, state)) {
return false;
}

// Always auto-create system indexes
if (systemIndices.isSystemIndex(index)) {
return true;
}

// One volatile read, so that all checks are done against the same instance:
final AutoCreate autoCreate = this.autoCreate;
if (autoCreate.autoCreateIndex == false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.indices.IndicesModule;
import org.elasticsearch.indices.SystemIndices;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.node.InternalSettingsPreparer;
import org.elasticsearch.node.Node;
Expand All @@ -67,14 +68,17 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySet;
import static java.util.Collections.unmodifiableList;
import static java.util.stream.Collectors.toList;
import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds;

Expand Down Expand Up @@ -150,18 +154,18 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
final List<Closeable> resourcesToClose = new ArrayList<>();
final ThreadPool threadPool = new ThreadPool(settings);
resourcesToClose.add(() -> ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS));
final NetworkService networkService = new NetworkService(Collections.emptyList());
final NetworkService networkService = new NetworkService(emptyList());
try {
final List<Setting<?>> additionalSettings = new ArrayList<>(pluginsService.getPluginSettings());
final List<String> additionalSettingsFilter = new ArrayList<>(pluginsService.getPluginSettingsFilter());
for (final ExecutorBuilder<?> builder : threadPool.builders()) {
additionalSettings.addAll(builder.getRegisteredSettings());
}
SettingsModule settingsModule =
new SettingsModule(settings, additionalSettings, additionalSettingsFilter, Collections.emptySet());
new SettingsModule(settings, additionalSettings, additionalSettingsFilter, emptySet());

SearchModule searchModule = new SearchModule(settings, true, pluginsService.filterPlugins(SearchPlugin.class));
IndicesModule indicesModule = new IndicesModule(Collections.emptyList());
IndicesModule indicesModule = new IndicesModule(emptyList());
List<NamedWriteableRegistry.Entry> entries = new ArrayList<>();
entries.addAll(NetworkModule.getNamedWriteables());
entries.addAll(searchModule.getNamedWriteables());
Expand All @@ -185,11 +189,11 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
modules.add(b -> b.bind(ThreadPool.class).toInstance(threadPool));
ActionModule actionModule = new ActionModule(true, settings, null, settingsModule.getIndexScopedSettings(),
settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(), threadPool,
pluginsService.filterPlugins(ActionPlugin.class), null, null, null);
pluginsService.filterPlugins(ActionPlugin.class), null, null, null, new SystemIndices(emptyMap()));
modules.add(actionModule);

CircuitBreakerService circuitBreakerService = Node.createCircuitBreakerService(settingsModule.getSettings(),
Collections.emptyList(),
emptyList(),
settingsModule.getClusterSettings());
resourcesToClose.add(circuitBreakerService);
PageCacheRecycler pageCacheRecycler = new PageCacheRecycler(settings);
Expand All @@ -202,7 +206,7 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
final TransportService transportService = new TransportService(settings, transport, threadPool,
networkModule.getTransportInterceptor(),
boundTransportAddress -> DiscoveryNode.createLocal(settings, new TransportAddress(TransportAddress.META_ADDRESS, 0),
UUIDs.randomBase64UUID()), null, Collections.emptySet());
UUIDs.randomBase64UUID()), null, emptySet());
modules.add((b -> {
b.bind(BigArrays.class).toInstance(bigArrays);
b.bind(PageCacheRecycler.class).toInstance(pageCacheRecycler);
Expand Down Expand Up @@ -300,7 +304,7 @@ protected TransportClient(Settings settings, Settings defaultSettings, Collectio
private TransportClient(ClientTemplate template) {
super(template.getSettings(), template.getThreadPool());
this.injector = template.injector;
this.pluginLifecycleComponents = Collections.unmodifiableList(template.pluginLifecycleComponents);
this.pluginLifecycleComponents = unmodifiableList(template.pluginLifecycleComponents);
this.nodesService = template.nodesService;
this.proxy = template.proxy;
this.namedWriteableRegistry = template.namedWriteableRegistry;
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ protected Node(final Environment initialEnvironment,

ActionModule actionModule = new ActionModule(false, settings, clusterModule.getIndexNameExpressionResolver(),
settingsModule.getIndexScopedSettings(), settingsModule.getClusterSettings(), settingsModule.getSettingsFilter(),
threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService);
threadPool, pluginsService.filterPlugins(ActionPlugin.class), client, circuitBreakerService, usageService, systemIndices);
modules.add(actionModule);

final RestController restController = actionModule.getRestController();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() {
UsageService usageService = new UsageService();
ActionModule actionModule = new ActionModule(false, settings.getSettings(), new IndexNameExpressionResolver(),
settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), null, emptyList(), null,
null, usageService);
null, usageService, null);
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, () ->
Expand Down Expand Up @@ -148,7 +148,7 @@ public String getName() {
UsageService usageService = new UsageService();
ActionModule actionModule = new ActionModule(false, settings.getSettings(), new IndexNameExpressionResolver(),
settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool,
singletonList(dupsMainAction), null, null, usageService);
singletonList(dupsMainAction), null, null, usageService, null);
Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null));
assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET"));
} finally {
Expand Down Expand Up @@ -182,7 +182,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
UsageService usageService = new UsageService();
ActionModule actionModule = new ActionModule(false, settings.getSettings(), new IndexNameExpressionResolver(),
settings.getIndexScopedSettings(), settings.getClusterSettings(), settings.getSettingsFilter(), threadPool,
singletonList(registersFakeHandler), null, null, usageService);
singletonList(registersFakeHandler), null, null, usageService, null);
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, () ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ class TestTransportBulkAction extends TransportBulkAction {
null, null, new ActionFilters(Collections.emptySet()), null,
new AutoCreateIndex(
SETTINGS, new ClusterSettings(SETTINGS, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new IndexNameExpressionResolver()
new IndexNameExpressionResolver(),
new SystemIndices(emptyMap())
), new IndexingPressure(SETTINGS), new SystemIndices(emptyMap())
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class TestTransportBulkAction extends TransportBulkAction {
TestTransportBulkAction() {
super(TransportBulkActionTests.this.threadPool, transportService, clusterService, null, null,
null, new ActionFilters(Collections.emptySet()), new Resolver(),
new AutoCreateIndex(Settings.EMPTY, clusterService.getClusterSettings(), new Resolver()),
new AutoCreateIndex(Settings.EMPTY, clusterService.getClusterSettings(), new Resolver(), new SystemIndices(emptyMap())),
new IndexingPressure(Settings.EMPTY), new SystemIndices(emptyMap()));
}

Expand Down Expand Up @@ -271,6 +271,28 @@ public void testOnlySystem() {
assertFalse(bulkAction.isOnlySystem(buildBulkRequest(mixed), indicesLookup, systemIndices));
}

public void testIncludesSystem() {
SortedMap<String, IndexAbstraction> indicesLookup = new TreeMap<>();
Settings settings = Settings.builder().put("index.version.created", Version.CURRENT).build();
indicesLookup.put(".foo",
new Index(IndexMetadata.builder(".foo").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build()));
indicesLookup.put(".bar",
new Index(IndexMetadata.builder(".bar").settings(settings).system(true).numberOfShards(1).numberOfReplicas(0).build()));
SystemIndices systemIndices = new SystemIndices(org.elasticsearch.common.collect.Map.of("plugin",
org.elasticsearch.common.collect.List.of(new SystemIndexDescriptor(".test", ""))));
List<String> onlySystem = org.elasticsearch.common.collect.List.of(".foo", ".bar");
assertTrue(bulkAction.includesSystem(buildBulkRequest(onlySystem), indicesLookup, systemIndices));

onlySystem = org.elasticsearch.common.collect.List.of(".foo", ".bar", ".test");
assertTrue(bulkAction.includesSystem(buildBulkRequest(onlySystem), indicesLookup, systemIndices));

List<String> nonSystem = org.elasticsearch.common.collect.List.of("foo", "bar");
assertFalse(bulkAction.includesSystem(buildBulkRequest(nonSystem), indicesLookup, systemIndices));

List<String> mixed = org.elasticsearch.common.collect.List.of(".foo", ".test", "other");
assertTrue(bulkAction.includesSystem(buildBulkRequest(mixed), indicesLookup, systemIndices));
}

private BulkRequest buildBulkRequest(List<String> indices) {
BulkRequest request = new BulkRequest();
for (String index : indices) {
Expand Down
Loading

0 comments on commit 6899ce6

Please sign in to comment.