Validate that system indices aren't also hidden indices

server/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java

@@ -92,6 +92,18 @@ protected void masterOperation(Task task, final UpdateSettingsRequest request, f
             return;
         }
 
+        final List<String> 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)
@@ -145,4 +157,31 @@ private Map<String, List<String>> 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<String> 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 List.of();
+        }
+
+        final List<String> systemPatterns = new ArrayList<>();
+
+        for (Index index : concreteIndices) {
+            final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(index.getName());
+            if (descriptor != null) {
+                systemPatterns.add(index.getName());
+            }
+        }
+
+        return systemPatterns;
+    }
 }

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());
                     }
                 }
             }

server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java

@@ -259,6 +259,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;

server/src/test/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsActionTests.java

@@ -0,0 +1,105 @@
+/*
+ * 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.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.indices.SystemIndices;
+import org.elasticsearch.tasks.Task;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.transport.TransportService;
+import org.junit.Before;
+import org.mockito.ArgumentCaptor;
+
+import java.util.List;
+import java.util.Map;
+
+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<AcknowledgedResponse> mockListener = mock(ActionListener.class);
+
+        action.masterOperation(mock(Task.class), request, CLUSTER_STATE, mockListener);
+
+        ArgumentCaptor<Exception> 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]"));
+    }
+}

server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java

@@ -0,0 +1,101 @@
+/*
+ * 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.settings.Settings;
+import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.indices.SystemIndices;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.Before;
+
+import java.util.List;
+import java.util.Map;
+
+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);
+    }
+}

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*")