From 85728e473b90b35bd1f1a76edf001d182329d1ad Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 5 May 2021 13:38:17 -0400 Subject: [PATCH 1/7] Validate that system indices aren't also hidden inidices --- .../SystemIndexMetadataUpgradeService.java | 8 +++++++ .../indices/SystemIndexDescriptor.java | 5 +++++ .../indices/SystemIndexDescriptorTests.java | 22 +++++++++++++++++++ 3 files changed, 35 insertions(+) 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..6d8c86bfe807a 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java @@ -81,6 +81,14 @@ public ClusterState execute(ClusterState currentState) throws Exception { if (isSystem != cursor.value.isSystem()) { updatedMetadata.add(IndexMetadata.builder(cursor.value).system(cursor.value.isSystem() == false).build()); } + + // TODO: do we have test coverage for this? + if (isSystem) { + if (cursor.value.getSettings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) { + throw new IllegalStateException("Index " + cursor.value.getIndex().getName() + "is a system " + + "index, but has the [index.hidden] setting set to true."); + } + } } } diff --git a/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java b/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java index a5912ead4b240..17449d7174dff 100644 --- a/server/src/main/java/org/elasticsearch/indices/SystemIndexDescriptor.java +++ b/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; diff --git a/server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java b/server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java index 76810f067ed9e..8e203d4dbb29e 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*") From ba0bca79442ab7ff8e2deeed65edb38142c62d34 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 5 May 2021 14:03:52 -0400 Subject: [PATCH 2/7] Remove hidden from ingest geo system index --- .../java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java index 5a97678599634..d61a374dc224c 100644 --- a/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java +++ b/modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/IngestGeoIpPlugin.java @@ -166,7 +166,6 @@ public Collection getSystemIndexDescriptors(Settings sett .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1") - .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) .build()) .setOrigin("geoip") .setVersionMetaKey("version") From 5b04bcfd3f6d5871faaed7fee27401473c9e0d5f Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 6 May 2021 23:06:16 -0400 Subject: [PATCH 3/7] Add test coverage --- .../SystemIndexMetadataUpgradeService.java | 10 ++- ...ystemIndexMetadataUpgradeServiceTests.java | 78 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java 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 6d8c86bfe807a..8fad77b57dbb7 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java @@ -68,6 +68,11 @@ public void clusterChanged(ClusterChangedEvent event) { } } + // visible for testing + SystemIndexMetadataUpdateTask getTask() { + return new SystemIndexMetadataUpdateTask(); + } + public class SystemIndexMetadataUpdateTask extends ClusterStateUpdateTask { @Override @@ -82,11 +87,10 @@ public ClusterState execute(ClusterState currentState) throws Exception { updatedMetadata.add(IndexMetadata.builder(cursor.value).system(cursor.value.isSystem() == false).build()); } - // TODO: do we have test coverage for this? if (isSystem) { if (cursor.value.getSettings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) { - throw new IllegalStateException("Index " + cursor.value.getIndex().getName() + "is a system " + - "index, but has the [index.hidden] setting set to true."); + throw new IllegalStateException("Cannot define index [" + cursor.value.getIndex().getName() + + "] as a system index because it has the [index.hidden] setting set to true."); } } } 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..8df8efeff8f5c --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java @@ -0,0 +1,78 @@ +/* + * 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 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(); + + /** + * When we upgrade Elasticsearch versions, existing indices may be newly + * defined as system indices. We need to validate that these indices don't + * have the "hidden index" setting. + */ + public void testMakingHiddenIndexIntoSystemIndexFails() { + // set up a system index upgrade service + SystemIndexMetadataUpgradeService service = new SystemIndexMetadataUpgradeService( + new SystemIndices( + Map.of("MyIndex", new SystemIndices.Feature("foo", "a test feature", List.of(DESCRIPTOR)))), + mock(ClusterService.class) + ); + + // create an initial cluster state with a hidden index that matches the system index descriptor + IndexMetadata.Builder hiddenIndexMetadata = IndexMetadata.builder(SYSTEM_INDEX_NAME) + .settings(getSettingsBuilder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true)); + + 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 + IllegalStateException exception = expectThrows(IllegalStateException.class, + () -> service.getTask().execute(clusterState)); + + assertThat(exception.getMessage(), + equalTo("Cannot define index [.myindex-1] as a system index because it has the [index.hidden] setting set to true.")); + } + + 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); + } +} From bf86c6c6a4edbb6ece8905e62c0b4d375caaeb3a Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 14 May 2021 17:25:40 -0400 Subject: [PATCH 4/7] Remove hidden setting if updating to become a system index --- .../SystemIndexMetadataUpgradeService.java | 16 +++++++++------- .../SystemIndexMetadataUpgradeServiceTests.java | 14 +++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) 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 8fad77b57dbb7..981328c1eb718 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; @@ -83,16 +84,17 @@ 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); if (isSystem != cursor.value.isSystem()) { - updatedMetadata.add(IndexMetadata.builder(cursor.value).system(cursor.value.isSystem() == false).build()); - } - - if (isSystem) { - if (cursor.value.getSettings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) { - throw new IllegalStateException("Cannot define index [" + cursor.value.getIndex().getName() + - "] as a system index because it has the [index.hidden] setting set to true."); + builder.system(cursor.value.isSystem() == false); + 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)); } + updatedMetadata.add(builder.build()); } + } } diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java index 8df8efeff8f5c..aa6e83bb63f69 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java @@ -39,10 +39,10 @@ public class SystemIndexMetadataUpgradeServiceTests extends ESTestCase { /** * When we upgrade Elasticsearch versions, existing indices may be newly - * defined as system indices. We need to validate that these indices don't - * have the "hidden index" setting. + * defined as system indices. If such indices are set to "hidden," we need + * to remove that setting. */ - public void testMakingHiddenIndexIntoSystemIndexFails() { + public void testUpgradeHiddenIndexToSystemIndex() throws Exception { // set up a system index upgrade service SystemIndexMetadataUpgradeService service = new SystemIndexMetadataUpgradeService( new SystemIndices( @@ -62,11 +62,11 @@ public void testMakingHiddenIndexIntoSystemIndexFails() { .customs(ImmutableOpenMap.of()).build(); // Get a metadata upgrade task and execute it on the initial cluster state - IllegalStateException exception = expectThrows(IllegalStateException.class, - () -> service.getTask().execute(clusterState)); + ClusterState newState = service.getTask().execute(clusterState); - assertThat(exception.getMessage(), - equalTo("Cannot define index [.myindex-1] as a system index because it has the [index.hidden] setting set to true.")); + 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() { From a87e2c20693bb87bbfe8c5515645dd80048199ba Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 14 May 2021 17:26:18 -0400 Subject: [PATCH 5/7] Reject settings updates that try to hide system indices --- .../put/TransportUpdateSettingsAction.java | 39 +++++++ .../TransportUpdateSettingsActionTests.java | 105 ++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsActionTests.java 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 2ff216a02a0b1..4859edab7ac76 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 @@ -92,6 +92,18 @@ protected void masterOperation(Task task, final UpdateSettingsRequest request, f return; } + 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) @@ -145,4 +157,31 @@ private Map> checkForSystemIndexViolations(Index[] concrete return violationsByIndex; } + + /** + * Checks that if the request is trying to apply settings changes to any system indices, then the settings' values match those + * that the system index's descriptor expects. + * + * @param concreteIndices the indices being updated + * @param request the update request + * @return a mapping from system index pattern to the settings whose values would be overridden. Empty if there are no violations. + */ + 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 List.of(); + } + + 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/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..6f96c2cd9f857 --- /dev/null +++ b/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 mockListener = mock(ActionListener.class); + + action.masterOperation(mock(Task.class), 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]")); + } +} From c94e62c7a310e1a0c1a4e5044798a2909d68af98 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Fri, 14 May 2021 17:41:29 -0400 Subject: [PATCH 6/7] Update javadoc --- .../indices/settings/put/TransportUpdateSettingsAction.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 4859edab7ac76..7c21a31c9f80e 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 @@ -159,12 +159,12 @@ private Map> checkForSystemIndexViolations(Index[] concrete } /** - * Checks that if the request is trying to apply settings changes to any system indices, then the settings' values match those - * that the system index's descriptor expects. + * 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 mapping from system index pattern to the settings whose values would be overridden. Empty if there are no violations. + * @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 From 064cdc89fffbd3223d9d4f9c5214a132b0ed973a Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 25 May 2021 13:12:46 -0400 Subject: [PATCH 7/7] Remove hidden setting from system index even if not upgrading --- .../SystemIndexMetadataUpgradeService.java | 16 +++++---- ...ystemIndexMetadataUpgradeServiceTests.java | 35 +++++++++++++++---- 2 files changed, 39 insertions(+), 12 deletions(-) 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 981328c1eb718..4fd926218f1c3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java @@ -85,16 +85,20 @@ public ClusterState execute(ClusterState currentState) throws Exception { 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()) { builder.system(cursor.value.isSystem() == false); - 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 (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/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java index aa6e83bb63f69..8b4784f20c497 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeServiceTests.java @@ -17,6 +17,7 @@ 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; @@ -37,23 +38,45 @@ public class SystemIndexMetadataUpgradeServiceTests extends ESTestCase { .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 { - // set up a system index upgrade service - SystemIndexMetadataUpgradeService service = new SystemIndexMetadataUpgradeService( - new SystemIndices( - Map.of("MyIndex", new SystemIndices.Feature("foo", "a test feature", List.of(DESCRIPTOR)))), - mock(ClusterService.class) - ); + // 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);