Skip to content

Commit 9035e61

Browse files
tvernumtvernum
authored
Detect when security index is closed (#42740)
If the security index is closed, it should be treated as unavailable for security purposes. Prior to 8.0 (or in a mixed cluster) a closed security index has no routing data, which would cause a NPE in the cluster change handler, and the index state would not be updated correctly. This commit fixes that problem Backport of: #42191
1 parent 87cc6a9 commit 9035e61

File tree

6 files changed

+119
-61
lines changed

6 files changed

+119
-61
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java‎

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@
4545
import org.elasticsearch.common.xcontent.XContentType;
4646
import org.elasticsearch.common.xcontent.json.JsonXContent;
4747
import org.elasticsearch.gateway.GatewayService;
48+
import org.elasticsearch.index.Index;
4849
import org.elasticsearch.index.IndexNotFoundException;
4950
import org.elasticsearch.index.mapper.MapperService;
51+
import org.elasticsearch.indices.IndexClosedException;
5052
import org.elasticsearch.rest.RestStatus;
5153
import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames;
5254
import org.elasticsearch.xpack.core.template.TemplateUtils;
@@ -173,9 +175,11 @@ public ElasticsearchException getUnavailableReason() {
173175
throw new IllegalStateException("caller must make sure to use a frozen state and check indexAvailable");
174176
}
175177

176-
if (localState.indexExists()) {
178+
if (localState.indexState == IndexMetaData.State.CLOSE) {
179+
return new IndexClosedException(new Index(localState.concreteIndexName, ClusterState.UNKNOWN_UUID));
180+
} else if (localState.indexExists()) {
177181
return new UnavailableShardsException(null,
178-
"at least one primary shard for the index [" + localState.concreteIndexName + "] is unavailable");
182+
"at least one primary shard for the index [" + localState.concreteIndexName + "] is unavailable");
179183
} else {
180184
return new IndexNotFoundException(localState.concreteIndexName);
181185
}
@@ -206,11 +210,24 @@ public void clusterChanged(ClusterChangedEvent event) {
206210
final boolean indexAvailable = checkIndexAvailable(event.state());
207211
final boolean mappingIsUpToDate = indexMetaData == null || checkIndexMappingUpToDate(event.state());
208212
final Version mappingVersion = oldestIndexMappingVersion(event.state());
209-
final ClusterHealthStatus indexStatus = indexMetaData == null ? null :
210-
new ClusterIndexHealth(indexMetaData, event.state().getRoutingTable().index(indexMetaData.getIndex())).getStatus();
211213
final String concreteIndexName = indexMetaData == null ? internalIndexName : indexMetaData.getIndex().getName();
214+
final ClusterHealthStatus indexHealth;
215+
final IndexMetaData.State indexState;
216+
if (indexMetaData == null) {
217+
// Index does not exist
218+
indexState = null;
219+
indexHealth = null;
220+
} else if (indexMetaData.getState() == IndexMetaData.State.CLOSE) {
221+
indexState = IndexMetaData.State.CLOSE;
222+
indexHealth = null;
223+
logger.warn("Index [{}] is closed. This is likely to prevent security from functioning correctly", concreteIndexName);
224+
} else {
225+
indexState = IndexMetaData.State.OPEN;
226+
final IndexRoutingTable routingTable = event.state().getRoutingTable().index(indexMetaData.getIndex());
227+
indexHealth = new ClusterIndexHealth(indexMetaData, routingTable).getStatus();
228+
}
212229
final State newState = new State(creationTime, isIndexUpToDate, indexAvailable, mappingIsUpToDate, mappingVersion,
213-
concreteIndexName, indexStatus);
230+
concreteIndexName, indexHealth, indexState);
214231
this.indexState = newState;
215232

216233
if (newState.equals(previousState) == false) {
@@ -221,23 +238,21 @@ public void clusterChanged(ClusterChangedEvent event) {
221238
}
222239

223240
private boolean checkIndexAvailable(ClusterState state) {
224-
final IndexRoutingTable routingTable = getIndexRoutingTable(state);
225-
if (routingTable != null && routingTable.allPrimaryShardsActive()) {
226-
return true;
227-
}
228-
logger.debug("Index [{}] is not yet active", aliasName);
229-
return false;
230-
}
231-
232-
/**
233-
* Returns the routing-table for this index, or <code>null</code> if the index does not exist.
234-
*/
235-
private IndexRoutingTable getIndexRoutingTable(ClusterState clusterState) {
236-
IndexMetaData metaData = resolveConcreteIndex(aliasName, clusterState.metaData());
241+
IndexMetaData metaData = resolveConcreteIndex(aliasName, state.metaData());
237242
if (metaData == null) {
238-
return null;
243+
logger.debug("Index [{}] is not available - no metadata", aliasName);
244+
return false;
245+
}
246+
if (metaData.getState() == IndexMetaData.State.CLOSE) {
247+
logger.warn("Index [{}] is closed", aliasName);
248+
return false;
249+
}
250+
final IndexRoutingTable routingTable = state.routingTable().index(metaData.getIndex());
251+
if (routingTable == null || routingTable.allPrimaryShardsActive() == false) {
252+
logger.debug("Index [{}] is not yet active", aliasName);
253+
return false;
239254
} else {
240-
return clusterState.routingTable().index(metaData.getIndex());
255+
return true;
241256
}
242257
}
243258

@@ -402,15 +417,15 @@ public void onFailure(Exception e) {
402417
* Return true if the state moves from an unhealthy ("RED") index state to a healthy ("non-RED") state.
403418
*/
404419
public static boolean isMoveFromRedToNonRed(State previousState, State currentState) {
405-
return (previousState.indexStatus == null || previousState.indexStatus == ClusterHealthStatus.RED)
406-
&& currentState.indexStatus != null && currentState.indexStatus != ClusterHealthStatus.RED;
420+
return (previousState.indexHealth == null || previousState.indexHealth == ClusterHealthStatus.RED)
421+
&& currentState.indexHealth != null && currentState.indexHealth != ClusterHealthStatus.RED;
407422
}
408423

409424
/**
410425
* Return true if the state moves from the index existing to the index not existing.
411426
*/
412427
public static boolean isIndexDeleted(State previousState, State currentState) {
413-
return previousState.indexStatus != null && currentState.indexStatus == null;
428+
return previousState.indexHealth != null && currentState.indexHealth == null;
414429
}
415430

416431
private static byte[] readTemplateAsBytes(String templateName) {
@@ -440,24 +455,27 @@ private static Tuple<String, Settings> parseMappingAndSettingsFromTemplateBytes(
440455
* State of the security index.
441456
*/
442457
public static class State {
443-
public static final State UNRECOVERED_STATE = new State(null, false, false, false, null, null, null);
458+
public static final State UNRECOVERED_STATE = new State(null, false, false, false, null, null, null, null);
444459
public final Instant creationTime;
445460
public final boolean isIndexUpToDate;
446461
public final boolean indexAvailable;
447462
public final boolean mappingUpToDate;
448463
public final Version mappingVersion;
449464
public final String concreteIndexName;
450-
public final ClusterHealthStatus indexStatus;
465+
public final ClusterHealthStatus indexHealth;
466+
public final IndexMetaData.State indexState;
451467

452468
public State(Instant creationTime, boolean isIndexUpToDate, boolean indexAvailable,
453-
boolean mappingUpToDate, Version mappingVersion, String concreteIndexName, ClusterHealthStatus indexStatus) {
469+
boolean mappingUpToDate, Version mappingVersion, String concreteIndexName, ClusterHealthStatus indexHealth,
470+
IndexMetaData.State indexState) {
454471
this.creationTime = creationTime;
455472
this.isIndexUpToDate = isIndexUpToDate;
456473
this.indexAvailable = indexAvailable;
457474
this.mappingUpToDate = mappingUpToDate;
458475
this.mappingVersion = mappingVersion;
459476
this.concreteIndexName = concreteIndexName;
460-
this.indexStatus = indexStatus;
477+
this.indexHealth = indexHealth;
478+
this.indexState = indexState;
461479
}
462480

463481
@Override
@@ -471,7 +489,8 @@ public boolean equals(Object o) {
471489
mappingUpToDate == state.mappingUpToDate &&
472490
Objects.equals(mappingVersion, state.mappingVersion) &&
473491
Objects.equals(concreteIndexName, state.concreteIndexName) &&
474-
indexStatus == state.indexStatus;
492+
indexHealth == state.indexHealth &&
493+
indexState == state.indexState;
475494
}
476495

477496
public boolean indexExists() {
@@ -481,7 +500,7 @@ public boolean indexExists() {
481500
@Override
482501
public int hashCode() {
483502
return Objects.hash(creationTime, isIndexUpToDate, indexAvailable, mappingUpToDate, mappingVersion, concreteIndexName,
484-
indexStatus);
503+
indexHealth);
485504
}
486505
}
487506
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.action.update.UpdateRequestBuilder;
2626
import org.elasticsearch.client.Client;
2727
import org.elasticsearch.cluster.health.ClusterHealthStatus;
28+
import org.elasticsearch.cluster.metadata.IndexMetaData;
2829
import org.elasticsearch.cluster.service.ClusterService;
2930
import org.elasticsearch.common.SuppressForbidden;
3031
import org.elasticsearch.common.UUIDs;
@@ -364,7 +365,7 @@ public void testCacheClearOnSecurityIndexChange() {
364365

365366
// green to yellow or yellow to green
366367
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
367-
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
368+
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
368369
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
369370
service.onSecurityIndexStateChange(previousState, currentState);
370371
assertEquals(expectedInvalidation, service.getNumInvalidation());
@@ -1402,6 +1403,7 @@ private void setCompletedToTrue(AtomicBoolean completed) {
14021403
}
14031404

14041405
private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
1405-
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
1406+
return new SecurityIndexManager.State(
1407+
Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus, IndexMetaData.State.OPEN);
14061408
}
14071409
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.security.authc.esnative;
77

88
import org.elasticsearch.cluster.health.ClusterHealthStatus;
9+
import org.elasticsearch.cluster.metadata.IndexMetaData;
910
import org.elasticsearch.common.settings.Settings;
1011
import org.elasticsearch.common.util.concurrent.ThreadContext;
1112
import org.elasticsearch.env.TestEnvironment;
@@ -27,7 +28,8 @@ public class NativeRealmTests extends ESTestCase {
2728
RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_6, RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_7);
2829

2930
private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
30-
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
31+
return new SecurityIndexManager.State(
32+
Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus, IndexMetaData.State.OPEN);
3133
}
3234

3335
public void testCacheClearOnIndexHealthChange() {
@@ -72,7 +74,7 @@ void clearCache() {
7274

7375
// green to yellow or yellow to green
7476
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
75-
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
77+
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
7678
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
7779
nativeRealm.onSecurityIndexStateChange(previousState, currentState);
7880
assertEquals(expectedInvalidation, numInvalidation.get());

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.client.Client;
1111
import org.elasticsearch.cluster.ClusterName;
1212
import org.elasticsearch.cluster.health.ClusterHealthStatus;
13+
import org.elasticsearch.cluster.metadata.IndexMetaData;
1314
import org.elasticsearch.common.bytes.BytesArray;
1415
import org.elasticsearch.common.settings.Settings;
1516
import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -138,7 +139,12 @@ private String randomiseDn(String dn) {
138139
}
139140

140141
private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
141-
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
142+
return indexState(true, indexStatus);
143+
}
144+
145+
private SecurityIndexManager.State indexState(boolean isUpToDate, ClusterHealthStatus healthStatus) {
146+
return new SecurityIndexManager.State(
147+
Instant.now(), isUpToDate, true, true, null, concreteSecurityIndexName, healthStatus, IndexMetaData.State.OPEN);
142148
}
143149

144150
public void testCacheClearOnIndexHealthChange() {
@@ -172,7 +178,7 @@ public void testCacheClearOnIndexHealthChange() {
172178

173179
// green to yellow or yellow to green
174180
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
175-
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
181+
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
176182
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
177183
store.onSecurityIndexStateChange(previousState, currentState);
178184
assertEquals(expectedInvalidation, numInvalidation.get());
@@ -182,14 +188,10 @@ public void testCacheClearOnIndexOutOfDateChange() {
182188
final AtomicInteger numInvalidation = new AtomicInteger(0);
183189
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation, true);
184190

185-
store.onSecurityIndexStateChange(
186-
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null),
187-
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null));
191+
store.onSecurityIndexStateChange(indexState(false, null), indexState(true, null));
188192
assertEquals(1, numInvalidation.get());
189193

190-
store.onSecurityIndexStateChange(
191-
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null),
192-
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null));
194+
store.onSecurityIndexStateChange(indexState(true, null), indexState(false, null));
193195
assertEquals(2, numInvalidation.get());
194196
}
195197

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,12 @@ Settings.EMPTY, fileRolesStore, nativeRolesStore, reservedRolesStore, mock(Nativ
763763
}
764764

765765
private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
766-
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
766+
return dummyIndexState(true, indexStatus);
767+
}
768+
769+
public SecurityIndexManager.State dummyIndexState(boolean isIndexUpToDate, ClusterHealthStatus healthStatus) {
770+
return new SecurityIndexManager.State(
771+
Instant.now(), isIndexUpToDate, true, true, null, concreteSecurityIndexName, healthStatus, IndexMetaData.State.OPEN);
767772
}
768773

769774
public void testCacheClearOnIndexHealthChange() {
@@ -812,7 +817,7 @@ public void invalidateAll() {
812817

813818
// green to yellow or yellow to green
814819
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
815-
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
820+
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
816821
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
817822
compositeRolesStore.onSecurityIndexStateChange(previousState, currentState);
818823
assertEquals(expectedInvalidation, numInvalidation.get());
@@ -837,14 +842,10 @@ public void invalidateAll() {
837842
}
838843
};
839844

840-
compositeRolesStore.onSecurityIndexStateChange(
841-
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null),
842-
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null));
845+
compositeRolesStore.onSecurityIndexStateChange(dummyIndexState(false, null), dummyIndexState(true, null));
843846
assertEquals(1, numInvalidation.get());
844847

845-
compositeRolesStore.onSecurityIndexStateChange(
846-
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null),
847-
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null));
848+
compositeRolesStore.onSecurityIndexStateChange(dummyIndexState(true, null), dummyIndexState(false, null));
848849
assertEquals(2, numInvalidation.get());
849850
}
850851

0 commit comments

Comments
 (0)