diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java index d8b6d7ecb459c..3fb30b07994be 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java @@ -94,6 +94,18 @@ protected void masterOperation(final UpdateSettingsRequest request, final Cluste deprecationLogger.deprecate(DeprecationCategory.API, "open_system_index_access", message); } + final List hiddenSystemIndexViolations + = checkForHidingSystemIndex(concreteIndices, request); + if (hiddenSystemIndexViolations.isEmpty() == false) { + final String message = "Cannot set [index.hidden] to 'true' on system indices: " + + hiddenSystemIndexViolations.stream() + .map(entry -> "[" + entry + "]") + .collect(Collectors.joining(", ")); + logger.warn(message); + listener.onFailure(new IllegalStateException(message)); + return; + } + UpdateSettingsClusterStateUpdateRequest clusterStateUpdateRequest = new UpdateSettingsClusterStateUpdateRequest() .indices(concreteIndices) .settings(requestSettings) @@ -147,4 +159,31 @@ private Map> checkForSystemIndexViolations(Index[] concrete return violationsByIndex; } + + /** + * Checks that the request isn't trying to add the "hidden" setting to a system + * index + * + * @param concreteIndices the indices being updated + * @param request the update request + * @return a list of system indexes that this request would set to hidden + */ + private List checkForHidingSystemIndex(Index[] concreteIndices, UpdateSettingsRequest request) { + // Requests that a cluster generates itself are permitted to have a difference in settings + // so that rolling upgrade scenarios still work. We check this via the request's origin. + if (request.settings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false) == false) { + return Collections.emptyList(); + } + + final List systemPatterns = new ArrayList<>(); + + for (Index index : concreteIndices) { + final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(index.getName()); + if (descriptor != null) { + systemPatterns.add(index.getName()); + } + } + + return systemPatterns; + } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java index 1e0144fe3e39d..4fd926218f1c3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java @@ -17,6 +17,7 @@ import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.indices.SystemIndices; import java.util.ArrayList; @@ -68,6 +69,11 @@ public void clusterChanged(ClusterChangedEvent event) { } } + // visible for testing + SystemIndexMetadataUpdateTask getTask() { + return new SystemIndexMetadataUpdateTask(); + } + public class SystemIndexMetadataUpdateTask extends ClusterStateUpdateTask { @Override @@ -78,8 +84,20 @@ public ClusterState execute(ClusterState currentState) throws Exception { if (cursor.value != lastIndexMetadataMap.get(cursor.key)) { final boolean isSystem = systemIndices.isSystemIndex(cursor.value.getIndex()) || systemIndices.isSystemIndexBackingDataStream(cursor.value.getIndex().getName()); + IndexMetadata.Builder builder = IndexMetadata.builder(cursor.value); + boolean updated = false; if (isSystem != cursor.value.isSystem()) { - updatedMetadata.add(IndexMetadata.builder(cursor.value).system(cursor.value.isSystem() == false).build()); + builder.system(cursor.value.isSystem() == false); + updated = true; + } + if (isSystem && cursor.value.getSettings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) { + builder.settings(Settings.builder() + .put(cursor.value.getSettings()) + .put(IndexMetadata.SETTING_INDEX_HIDDEN, false)); + updated = true; + } + if (updated) { + updatedMetadata.add(builder.build()); } } } diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java b/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java index 36efa83eb6b0d..bd0f6dc8d277f 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java +++ b/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java @@ -271,6 +271,11 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type, this.description = description; this.mappings = mappings; + + if (Objects.nonNull(settings) && settings.getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) { + throw new IllegalArgumentException("System indices cannot have " + IndexMetadata.SETTING_INDEX_HIDDEN + + " set to true."); + } this.settings = settings; this.indexFormat = indexFormat; this.versionMetaKey = versionMetaKey; diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsActionTests.java new file mode 100644 index 0000000000000..410e55554fbf5 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsActionTests.java @@ -0,0 +1,103 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.admin.indices.settings.put; + +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.metadata.MetadataUpdateSettingsService; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.collect.List; +import org.elasticsearch.common.collect.Map; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.indices.SystemIndices; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.TransportService; +import org.junit.Before; +import org.mockito.ArgumentCaptor; + +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +public class TransportUpdateSettingsActionTests extends ESTestCase { + + private static final ClusterState CLUSTER_STATE = ClusterState.builder(new ClusterName("test")) + .metadata(Metadata.builder() + .put(IndexMetadata.builder(".my-system") + .system(true) + .settings(Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .build()) + .build(), true) + .build()) + .build(); + + private static final String SYSTEM_INDEX_NAME = ".my-system"; + private static final SystemIndices SYSTEM_INDICES = new SystemIndices( + Map.of("test-feature", new SystemIndices.Feature( + "test-feature", + "a test feature", + List.of(new SystemIndexDescriptor(SYSTEM_INDEX_NAME, "test")) + )) + ); + + private TransportUpdateSettingsAction action; + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, SYSTEM_INDICES); + MetadataUpdateSettingsService metadataUpdateSettingsService = mock(MetadataUpdateSettingsService.class); + this.action = new TransportUpdateSettingsAction( + mock(TransportService.class), + mock(ClusterService.class), + null, + metadataUpdateSettingsService, + mock(ActionFilters.class), + indexNameExpressionResolver, + SYSTEM_INDICES + ); + } + + public void testSystemIndicesCannotBeSetToHidden() { + UpdateSettingsRequest request = new UpdateSettingsRequest( + Settings.builder() + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) + .build() + ); + request.indices(SYSTEM_INDEX_NAME); + + @SuppressWarnings("unchecked") + ActionListener mockListener = mock(ActionListener.class); + + action.masterOperation(request, CLUSTER_STATE, mockListener); + + ArgumentCaptor exceptionArgumentCaptor = ArgumentCaptor.forClass(Exception.class); + verify(mockListener, times(0)).onResponse(any()); + verify(mockListener, times(1)).onFailure(exceptionArgumentCaptor.capture()); + + Exception e = exceptionArgumentCaptor.getValue(); + assertThat(e.getMessage(), equalTo("Cannot set [index.hidden] to 'true' on system indices: [.my-system]")); + } +} diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java new file mode 100644 index 0000000000000..edb57f119c331 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java @@ -0,0 +1,100 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.cluster.metadata; + +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.collect.List; +import org.elasticsearch.common.collect.Map; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.indices.SystemIndices; +import org.elasticsearch.test.ESTestCase; +import org.junit.Before; + +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; + +public class SystemIndexMetadataUpgradeServiceTests extends ESTestCase { + + private static final String MAPPINGS = "{ \"_doc\": { \"_meta\": { \"version\": \"7.4.0\" } } }"; + private static final String SYSTEM_INDEX_NAME = ".myindex-1"; + private static final SystemIndexDescriptor DESCRIPTOR = SystemIndexDescriptor.builder() + .setIndexPattern(".myindex-*") + .setPrimaryIndex(SYSTEM_INDEX_NAME) + .setSettings(getSettingsBuilder().build()) + .setMappings(MAPPINGS) + .setVersionMetaKey("version") + .setOrigin("FAKE_ORIGIN") + .build(); + + private SystemIndexMetadataUpgradeService service; + + @Before + public void setUpTest() { + // set up a system index upgrade service + this.service = new SystemIndexMetadataUpgradeService( + new SystemIndices( + Map.of("MyIndex", new SystemIndices.Feature("foo", "a test feature", List.of(DESCRIPTOR)))), + mock(ClusterService.class) + ); + } + + /** + * When we upgrade Elasticsearch versions, existing indices may be newly + * defined as system indices. If such indices are set to "hidden," we need + * to remove that setting. + */ + public void testUpgradeHiddenIndexToSystemIndex() throws Exception { + // create an initial cluster state with a hidden index that matches the system index descriptor + IndexMetadata.Builder hiddenIndexMetadata = IndexMetadata.builder(SYSTEM_INDEX_NAME) + .system(false) + .settings(getSettingsBuilder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true)); + + assertSystemUpgradeRemovesHiddenSetting(hiddenIndexMetadata); + } + + /** + * If a system index erroneously is set to hidden, we should remedy that situation. + */ + public void testHiddenSettingRemovedFromSystemIndices() throws Exception { + // create an initial cluster state with a hidden index that matches the system index descriptor + IndexMetadata.Builder hiddenIndexMetadata = IndexMetadata.builder(SYSTEM_INDEX_NAME) + .system(true) + .settings(getSettingsBuilder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true)); + + assertSystemUpgradeRemovesHiddenSetting(hiddenIndexMetadata); + } + + private void assertSystemUpgradeRemovesHiddenSetting(IndexMetadata.Builder hiddenIndexMetadata) throws Exception { + Metadata.Builder clusterMetadata = new Metadata.Builder(); + clusterMetadata.put(hiddenIndexMetadata); + + ClusterState clusterState = ClusterState.builder(new ClusterName("system-index-metadata-upgrade-service-tests")) + .metadata(clusterMetadata.build()) + .customs(ImmutableOpenMap.of()).build(); + + // Get a metadata upgrade task and execute it on the initial cluster state + ClusterState newState = service.getTask().execute(clusterState); + + IndexMetadata result = newState.metadata().index(SYSTEM_INDEX_NAME); + assertThat(result.isSystem(), equalTo(true)); + assertThat(result.getSettings().get(IndexMetadata.SETTING_INDEX_HIDDEN), equalTo("false")); + } + + private static Settings.Builder getSettingsBuilder() { + return Settings.builder() + .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1); + } +} diff --git a/server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java b/server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java index 0b6ff984cd7b7..3de8f3cae8518 100644 --- a/server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java +++ b/server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.indices; import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -234,6 +235,27 @@ public void testGetDescriptorCompatibleWith() { assertSame(prior, compat); } + public void testSystemIndicesCannotAlsoBeHidden() { + SystemIndexDescriptor.Builder builder = SystemIndexDescriptor.builder() + .setIndexPattern(".system*") + .setDescription("system stuff") + .setPrimaryIndex(".system-1") + .setAliasName(".system") + .setType(Type.INTERNAL_MANAGED) + .setMappings(MAPPINGS) + .setVersionMetaKey("version") + .setOrigin("system"); + + builder.setSettings( + Settings.builder() + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) + .build()); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build); + + assertThat(e.getMessage(), equalTo("System indices cannot have index.hidden set to true.")); + } + private SystemIndexDescriptor.Builder priorSystemIndexDescriptorBuilder() { return SystemIndexDescriptor.builder() .setIndexPattern(".system*")