Skip to content

Commit ce362d7

Browse files
williamrandolphwilliamrandolph
committed
Validate that system indices aren't also hidden indices (#72768) (#73428)
* Validate that system indices aren't also hidden indices (#72768) * Validate that system indices aren't also hidden inidices * Remove hidden from ingest geo system index * Add test coverage * Remove hidden setting from system index even if not upgrading * Fix compilation for backport
1 parent addeb9f commit ce362d7

File tree

6 files changed

+288
-1
lines changed

6 files changed

+288
-1
lines changed

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,18 @@ protected void masterOperation(final UpdateSettingsRequest request, final Cluste
9494
deprecationLogger.deprecate(DeprecationCategory.API, "open_system_index_access", message);
9595
}
9696

97+
final List<String> hiddenSystemIndexViolations
98+
= checkForHidingSystemIndex(concreteIndices, request);
99+
if (hiddenSystemIndexViolations.isEmpty() == false) {
100+
final String message = "Cannot set [index.hidden] to 'true' on system indices: "
101+
+ hiddenSystemIndexViolations.stream()
102+
.map(entry -> "[" + entry + "]")
103+
.collect(Collectors.joining(", "));
104+
logger.warn(message);
105+
listener.onFailure(new IllegalStateException(message));
106+
return;
107+
}
108+
97109
UpdateSettingsClusterStateUpdateRequest clusterStateUpdateRequest = new UpdateSettingsClusterStateUpdateRequest()
98110
.indices(concreteIndices)
99111
.settings(requestSettings)
@@ -147,4 +159,31 @@ private Map<String, List<String>> checkForSystemIndexViolations(Index[] concrete
147159

148160
return violationsByIndex;
149161
}
162+
163+
/**
164+
* Checks that the request isn't trying to add the "hidden" setting to a system
165+
* index
166+
*
167+
* @param concreteIndices the indices being updated
168+
* @param request the update request
169+
* @return a list of system indexes that this request would set to hidden
170+
*/
171+
private List<String> checkForHidingSystemIndex(Index[] concreteIndices, UpdateSettingsRequest request) {
172+
// Requests that a cluster generates itself are permitted to have a difference in settings
173+
// so that rolling upgrade scenarios still work. We check this via the request's origin.
174+
if (request.settings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false) == false) {
175+
return Collections.emptyList();
176+
}
177+
178+
final List<String> systemPatterns = new ArrayList<>();
179+
180+
for (Index index : concreteIndices) {
181+
final SystemIndexDescriptor descriptor = systemIndices.findMatchingDescriptor(index.getName());
182+
if (descriptor != null) {
183+
systemPatterns.add(index.getName());
184+
}
185+
}
186+
187+
return systemPatterns;
188+
}
150189
}

‎server/src/main/java/org/elasticsearch/cluster/metadata/SystemIndexMetadataUpgradeService.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.cluster.ClusterStateUpdateTask;
1818
import org.elasticsearch.cluster.service.ClusterService;
1919
import org.elasticsearch.common.collect.ImmutableOpenMap;
20+
import org.elasticsearch.common.settings.Settings;
2021
import org.elasticsearch.indices.SystemIndices;
2122

2223
import java.util.ArrayList;
@@ -68,6 +69,11 @@ public void clusterChanged(ClusterChangedEvent event) {
6869
}
6970
}
7071

72+
// visible for testing
73+
SystemIndexMetadataUpdateTask getTask() {
74+
return new SystemIndexMetadataUpdateTask();
75+
}
76+
7177
public class SystemIndexMetadataUpdateTask extends ClusterStateUpdateTask {
7278

7379
@Override
@@ -78,8 +84,20 @@ public ClusterState execute(ClusterState currentState) throws Exception {
7884
if (cursor.value != lastIndexMetadataMap.get(cursor.key)) {
7985
final boolean isSystem = systemIndices.isSystemIndex(cursor.value.getIndex()) ||
8086
systemIndices.isSystemIndexBackingDataStream(cursor.value.getIndex().getName());
87+
IndexMetadata.Builder builder = IndexMetadata.builder(cursor.value);
88+
boolean updated = false;
8189
if (isSystem != cursor.value.isSystem()) {
82-
updatedMetadata.add(IndexMetadata.builder(cursor.value).system(cursor.value.isSystem() == false).build());
90+
builder.system(cursor.value.isSystem() == false);
91+
updated = true;
92+
}
93+
if (isSystem && cursor.value.getSettings().getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) {
94+
builder.settings(Settings.builder()
95+
.put(cursor.value.getSettings())
96+
.put(IndexMetadata.SETTING_INDEX_HIDDEN, false));
97+
updated = true;
98+
}
99+
if (updated) {
100+
updatedMetadata.add(builder.build());
83101
}
84102
}
85103
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ public SystemIndexDescriptor(String indexPattern, String description, Type type,
271271

272272
this.description = description;
273273
this.mappings = mappings;
274+
275+
if (Objects.nonNull(settings) && settings.getAsBoolean(IndexMetadata.SETTING_INDEX_HIDDEN, false)) {
276+
throw new IllegalArgumentException("System indices cannot have " + IndexMetadata.SETTING_INDEX_HIDDEN +
277+
" set to true.");
278+
}
274279
this.settings = settings;
275280
this.indexFormat = indexFormat;
276281
this.versionMetaKey = versionMetaKey;

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
package org.elasticsearch.action.admin.indices.settings.put;
10+
11+
import org.elasticsearch.Version;
12+
import org.elasticsearch.action.ActionListener;
13+
import org.elasticsearch.action.support.ActionFilters;
14+
import org.elasticsearch.action.support.master.AcknowledgedResponse;
15+
import org.elasticsearch.cluster.ClusterName;
16+
import org.elasticsearch.cluster.ClusterState;
17+
import org.elasticsearch.cluster.metadata.IndexMetadata;
18+
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
19+
import org.elasticsearch.cluster.metadata.Metadata;
20+
import org.elasticsearch.cluster.metadata.MetadataUpdateSettingsService;
21+
import org.elasticsearch.cluster.service.ClusterService;
22+
import org.elasticsearch.common.collect.List;
23+
import org.elasticsearch.common.collect.Map;
24+
import org.elasticsearch.common.settings.Settings;
25+
import org.elasticsearch.common.util.concurrent.ThreadContext;
26+
import org.elasticsearch.indices.SystemIndexDescriptor;
27+
import org.elasticsearch.indices.SystemIndices;
28+
import org.elasticsearch.test.ESTestCase;
29+
import org.elasticsearch.transport.TransportService;
30+
import org.junit.Before;
31+
import org.mockito.ArgumentCaptor;
32+
33+
import static org.hamcrest.Matchers.equalTo;
34+
import static org.mockito.Matchers.any;
35+
import static org.mockito.Mockito.mock;
36+
import static org.mockito.Mockito.times;
37+
import static org.mockito.Mockito.verify;
38+
39+
public class TransportUpdateSettingsActionTests extends ESTestCase {
40+
41+
private static final ClusterState CLUSTER_STATE = ClusterState.builder(new ClusterName("test"))
42+
.metadata(Metadata.builder()
43+
.put(IndexMetadata.builder(".my-system")
44+
.system(true)
45+
.settings(Settings.builder()
46+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
47+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
48+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
49+
.build())
50+
.build(), true)
51+
.build())
52+
.build();
53+
54+
private static final String SYSTEM_INDEX_NAME = ".my-system";
55+
private static final SystemIndices SYSTEM_INDICES = new SystemIndices(
56+
Map.of("test-feature", new SystemIndices.Feature(
57+
"test-feature",
58+
"a test feature",
59+
List.of(new SystemIndexDescriptor(SYSTEM_INDEX_NAME, "test"))
60+
))
61+
);
62+
63+
private TransportUpdateSettingsAction action;
64+
65+
@Before
66+
@Override
67+
public void setUp() throws Exception {
68+
super.setUp();
69+
ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
70+
IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(threadContext, SYSTEM_INDICES);
71+
MetadataUpdateSettingsService metadataUpdateSettingsService = mock(MetadataUpdateSettingsService.class);
72+
this.action = new TransportUpdateSettingsAction(
73+
mock(TransportService.class),
74+
mock(ClusterService.class),
75+
null,
76+
metadataUpdateSettingsService,
77+
mock(ActionFilters.class),
78+
indexNameExpressionResolver,
79+
SYSTEM_INDICES
80+
);
81+
}
82+
83+
public void testSystemIndicesCannotBeSetToHidden() {
84+
UpdateSettingsRequest request = new UpdateSettingsRequest(
85+
Settings.builder()
86+
.put(IndexMetadata.SETTING_INDEX_HIDDEN, true)
87+
.build()
88+
);
89+
request.indices(SYSTEM_INDEX_NAME);
90+
91+
@SuppressWarnings("unchecked")
92+
ActionListener<AcknowledgedResponse> mockListener = mock(ActionListener.class);
93+
94+
action.masterOperation(request, CLUSTER_STATE, mockListener);
95+
96+
ArgumentCaptor<Exception> exceptionArgumentCaptor = ArgumentCaptor.forClass(Exception.class);
97+
verify(mockListener, times(0)).onResponse(any());
98+
verify(mockListener, times(1)).onFailure(exceptionArgumentCaptor.capture());
99+
100+
Exception e = exceptionArgumentCaptor.getValue();
101+
assertThat(e.getMessage(), equalTo("Cannot set [index.hidden] to 'true' on system indices: [.my-system]"));
102+
}
103+
}

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0 and the Server Side Public License, v 1; you may not use this file except
5+
* in compliance with, at your election, the Elastic License 2.0 or the Server
6+
* Side Public License, v 1.
7+
*/
8+
9+
package org.elasticsearch.cluster.metadata;
10+
11+
import org.elasticsearch.Version;
12+
import org.elasticsearch.cluster.ClusterName;
13+
import org.elasticsearch.cluster.ClusterState;
14+
import org.elasticsearch.cluster.service.ClusterService;
15+
import org.elasticsearch.common.collect.ImmutableOpenMap;
16+
import org.elasticsearch.common.collect.List;
17+
import org.elasticsearch.common.collect.Map;
18+
import org.elasticsearch.common.settings.Settings;
19+
import org.elasticsearch.indices.SystemIndexDescriptor;
20+
import org.elasticsearch.indices.SystemIndices;
21+
import org.elasticsearch.test.ESTestCase;
22+
import org.junit.Before;
23+
24+
import static org.hamcrest.Matchers.equalTo;
25+
import static org.mockito.Mockito.mock;
26+
27+
public class SystemIndexMetadataUpgradeServiceTests extends ESTestCase {
28+
29+
private static final String MAPPINGS = "{ \"_doc\": { \"_meta\": { \"version\": \"7.4.0\" } } }";
30+
private static final String SYSTEM_INDEX_NAME = ".myindex-1";
31+
private static final SystemIndexDescriptor DESCRIPTOR = SystemIndexDescriptor.builder()
32+
.setIndexPattern(".myindex-*")
33+
.setPrimaryIndex(SYSTEM_INDEX_NAME)
34+
.setSettings(getSettingsBuilder().build())
35+
.setMappings(MAPPINGS)
36+
.setVersionMetaKey("version")
37+
.setOrigin("FAKE_ORIGIN")
38+
.build();
39+
40+
private SystemIndexMetadataUpgradeService service;
41+
42+
@Before
43+
public void setUpTest() {
44+
// set up a system index upgrade service
45+
this.service = new SystemIndexMetadataUpgradeService(
46+
new SystemIndices(
47+
Map.of("MyIndex", new SystemIndices.Feature("foo", "a test feature", List.of(DESCRIPTOR)))),
48+
mock(ClusterService.class)
49+
);
50+
}
51+
52+
/**
53+
* When we upgrade Elasticsearch versions, existing indices may be newly
54+
* defined as system indices. If such indices are set to "hidden," we need
55+
* to remove that setting.
56+
*/
57+
public void testUpgradeHiddenIndexToSystemIndex() throws Exception {
58+
// create an initial cluster state with a hidden index that matches the system index descriptor
59+
IndexMetadata.Builder hiddenIndexMetadata = IndexMetadata.builder(SYSTEM_INDEX_NAME)
60+
.system(false)
61+
.settings(getSettingsBuilder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true));
62+
63+
assertSystemUpgradeRemovesHiddenSetting(hiddenIndexMetadata);
64+
}
65+
66+
/**
67+
* If a system index erroneously is set to hidden, we should remedy that situation.
68+
*/
69+
public void testHiddenSettingRemovedFromSystemIndices() throws Exception {
70+
// create an initial cluster state with a hidden index that matches the system index descriptor
71+
IndexMetadata.Builder hiddenIndexMetadata = IndexMetadata.builder(SYSTEM_INDEX_NAME)
72+
.system(true)
73+
.settings(getSettingsBuilder().put(IndexMetadata.SETTING_INDEX_HIDDEN, true));
74+
75+
assertSystemUpgradeRemovesHiddenSetting(hiddenIndexMetadata);
76+
}
77+
78+
private void assertSystemUpgradeRemovesHiddenSetting(IndexMetadata.Builder hiddenIndexMetadata) throws Exception {
79+
Metadata.Builder clusterMetadata = new Metadata.Builder();
80+
clusterMetadata.put(hiddenIndexMetadata);
81+
82+
ClusterState clusterState = ClusterState.builder(new ClusterName("system-index-metadata-upgrade-service-tests"))
83+
.metadata(clusterMetadata.build())
84+
.customs(ImmutableOpenMap.of()).build();
85+
86+
// Get a metadata upgrade task and execute it on the initial cluster state
87+
ClusterState newState = service.getTask().execute(clusterState);
88+
89+
IndexMetadata result = newState.metadata().index(SYSTEM_INDEX_NAME);
90+
assertThat(result.isSystem(), equalTo(true));
91+
assertThat(result.getSettings().get(IndexMetadata.SETTING_INDEX_HIDDEN), equalTo("false"));
92+
}
93+
94+
private static Settings.Builder getSettingsBuilder() {
95+
return Settings.builder()
96+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
97+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
98+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1);
99+
}
100+
}

‎server/src/test/java/org/elasticsearch/indices/SystemIndexDescriptorTests.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package org.elasticsearch.indices;
1010

1111
import org.elasticsearch.Version;
12+
import org.elasticsearch.cluster.metadata.IndexMetadata;
1213
import org.elasticsearch.common.settings.Settings;
1314
import org.elasticsearch.common.xcontent.XContentHelper;
1415
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -234,6 +235,27 @@ public void testGetDescriptorCompatibleWith() {
234235
assertSame(prior, compat);
235236
}
236237

238+
public void testSystemIndicesCannotAlsoBeHidden() {
239+
SystemIndexDescriptor.Builder builder = SystemIndexDescriptor.builder()
240+
.setIndexPattern(".system*")
241+
.setDescription("system stuff")
242+
.setPrimaryIndex(".system-1")
243+
.setAliasName(".system")
244+
.setType(Type.INTERNAL_MANAGED)
245+
.setMappings(MAPPINGS)
246+
.setVersionMetaKey("version")
247+
.setOrigin("system");
248+
249+
builder.setSettings(
250+
Settings.builder()
251+
.put(IndexMetadata.SETTING_INDEX_HIDDEN, true)
252+
.build());
253+
254+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, builder::build);
255+
256+
assertThat(e.getMessage(), equalTo("System indices cannot have index.hidden set to true."));
257+
}
258+
237259
private SystemIndexDescriptor.Builder priorSystemIndexDescriptorBuilder() {
238260
return SystemIndexDescriptor.builder()
239261
.setIndexPattern(".system*")

0 commit comments

Comments
 (0)