Skip to content

Commit

Permalink
Detect when security index is closed (elastic#42191)
Browse files Browse the repository at this point in the history
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 fixese that problem
  • Loading branch information
tvernum authored May 30, 2019
1 parent 28ad74f commit f32f7d8
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.gateway.GatewayService;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.indices.IndexClosedException;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames;
import org.elasticsearch.xpack.core.template.TemplateUtils;
Expand Down Expand Up @@ -168,9 +170,11 @@ public ElasticsearchException getUnavailableReason() {
throw new IllegalStateException("caller must make sure to use a frozen state and check indexAvailable");
}

if (localState.indexExists()) {
if (localState.indexState == IndexMetaData.State.CLOSE) {
return new IndexClosedException(new Index(localState.concreteIndexName, ClusterState.UNKNOWN_UUID));
} else if (localState.indexExists()) {
return new UnavailableShardsException(null,
"at least one primary shard for the index [" + localState.concreteIndexName + "] is unavailable");
"at least one primary shard for the index [" + localState.concreteIndexName + "] is unavailable");
} else {
return new IndexNotFoundException(localState.concreteIndexName);
}
Expand Down Expand Up @@ -201,11 +205,24 @@ public void clusterChanged(ClusterChangedEvent event) {
final boolean indexAvailable = checkIndexAvailable(event.state());
final boolean mappingIsUpToDate = indexMetaData == null || checkIndexMappingUpToDate(event.state());
final Version mappingVersion = oldestIndexMappingVersion(event.state());
final ClusterHealthStatus indexStatus = indexMetaData == null ? null :
new ClusterIndexHealth(indexMetaData, event.state().getRoutingTable().index(indexMetaData.getIndex())).getStatus();
final String concreteIndexName = indexMetaData == null ? internalIndexName : indexMetaData.getIndex().getName();
final ClusterHealthStatus indexHealth;
final IndexMetaData.State indexState;
if (indexMetaData == null) {
// Index does not exist
indexState = null;
indexHealth = null;
} else if (indexMetaData.getState() == IndexMetaData.State.CLOSE) {
indexState = IndexMetaData.State.CLOSE;
indexHealth = null;
logger.warn("Index [{}] is closed. This is likely to prevent security from functioning correctly", concreteIndexName);
} else {
indexState = IndexMetaData.State.OPEN;
final IndexRoutingTable routingTable = event.state().getRoutingTable().index(indexMetaData.getIndex());
indexHealth = new ClusterIndexHealth(indexMetaData, routingTable).getStatus();
}
final State newState = new State(creationTime, isIndexUpToDate, indexAvailable, mappingIsUpToDate, mappingVersion,
concreteIndexName, indexStatus);
concreteIndexName, indexHealth, indexState);
this.indexState = newState;

if (newState.equals(previousState) == false) {
Expand All @@ -216,23 +233,21 @@ public void clusterChanged(ClusterChangedEvent event) {
}

private boolean checkIndexAvailable(ClusterState state) {
final IndexRoutingTable routingTable = getIndexRoutingTable(state);
if (routingTable != null && routingTable.allPrimaryShardsActive()) {
return true;
}
logger.debug("Index [{}] is not yet active", aliasName);
return false;
}

/**
* Returns the routing-table for this index, or <code>null</code> if the index does not exist.
*/
private IndexRoutingTable getIndexRoutingTable(ClusterState clusterState) {
IndexMetaData metaData = resolveConcreteIndex(aliasName, clusterState.metaData());
IndexMetaData metaData = resolveConcreteIndex(aliasName, state.metaData());
if (metaData == null) {
return null;
logger.debug("Index [{}] is not available - no metadata", aliasName);
return false;
}
if (metaData.getState() == IndexMetaData.State.CLOSE) {
logger.warn("Index [{}] is closed", aliasName);
return false;
}
final IndexRoutingTable routingTable = state.routingTable().index(metaData.getIndex());
if (routingTable == null || routingTable.allPrimaryShardsActive() == false) {
logger.debug("Index [{}] is not yet active", aliasName);
return false;
} else {
return clusterState.routingTable().index(metaData.getIndex());
return true;
}
}

Expand Down Expand Up @@ -397,15 +412,15 @@ public void onFailure(Exception e) {
* Return true if the state moves from an unhealthy ("RED") index state to a healthy ("non-RED") state.
*/
public static boolean isMoveFromRedToNonRed(State previousState, State currentState) {
return (previousState.indexStatus == null || previousState.indexStatus == ClusterHealthStatus.RED)
&& currentState.indexStatus != null && currentState.indexStatus != ClusterHealthStatus.RED;
return (previousState.indexHealth == null || previousState.indexHealth == ClusterHealthStatus.RED)
&& currentState.indexHealth != null && currentState.indexHealth != ClusterHealthStatus.RED;
}

/**
* Return true if the state moves from the index existing to the index not existing.
*/
public static boolean isIndexDeleted(State previousState, State currentState) {
return previousState.indexStatus != null && currentState.indexStatus == null;
return previousState.indexHealth != null && currentState.indexHealth == null;
}

private static byte[] readTemplateAsBytes(String templateName) {
Expand Down Expand Up @@ -435,24 +450,27 @@ private static Tuple<String, Settings> parseMappingAndSettingsFromTemplateBytes(
* State of the security index.
*/
public static class State {
public static final State UNRECOVERED_STATE = new State(null, false, false, false, null, null, null);
public static final State UNRECOVERED_STATE = new State(null, false, false, false, null, null, null, null);
public final Instant creationTime;
public final boolean isIndexUpToDate;
public final boolean indexAvailable;
public final boolean mappingUpToDate;
public final Version mappingVersion;
public final String concreteIndexName;
public final ClusterHealthStatus indexStatus;
public final ClusterHealthStatus indexHealth;
public final IndexMetaData.State indexState;

public State(Instant creationTime, boolean isIndexUpToDate, boolean indexAvailable,
boolean mappingUpToDate, Version mappingVersion, String concreteIndexName, ClusterHealthStatus indexStatus) {
boolean mappingUpToDate, Version mappingVersion, String concreteIndexName, ClusterHealthStatus indexHealth,
IndexMetaData.State indexState) {
this.creationTime = creationTime;
this.isIndexUpToDate = isIndexUpToDate;
this.indexAvailable = indexAvailable;
this.mappingUpToDate = mappingUpToDate;
this.mappingVersion = mappingVersion;
this.concreteIndexName = concreteIndexName;
this.indexStatus = indexStatus;
this.indexHealth = indexHealth;
this.indexState = indexState;
}

@Override
Expand All @@ -466,7 +484,8 @@ public boolean equals(Object o) {
mappingUpToDate == state.mappingUpToDate &&
Objects.equals(mappingVersion, state.mappingVersion) &&
Objects.equals(concreteIndexName, state.concreteIndexName) &&
indexStatus == state.indexStatus;
indexHealth == state.indexHealth &&
indexState == state.indexState;
}

public boolean indexExists() {
Expand All @@ -476,7 +495,7 @@ public boolean indexExists() {
@Override
public int hashCode() {
return Objects.hash(creationTime, isIndexUpToDate, indexAvailable, mappingUpToDate, mappingVersion, concreteIndexName,
indexStatus);
indexHealth);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.action.update.UpdateRequestBuilder;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.UUIDs;
Expand Down Expand Up @@ -364,7 +365,7 @@ public void testCacheClearOnSecurityIndexChange() {

// green to yellow or yellow to green
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
service.onSecurityIndexStateChange(previousState, currentState);
assertEquals(expectedInvalidation, service.getNumInvalidation());
Expand Down Expand Up @@ -1402,6 +1403,7 @@ private void setCompletedToTrue(AtomicBoolean completed) {
}

private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
return new SecurityIndexManager.State(
Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus, IndexMetaData.State.OPEN);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.elasticsearch.xpack.security.authc.esnative;

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

private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
return new SecurityIndexManager.State(
Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus, IndexMetaData.State.OPEN);
}

public void testCacheClearOnIndexHealthChange() {
Expand Down Expand Up @@ -72,7 +74,7 @@ void clearCache() {

// green to yellow or yellow to green
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
nativeRealm.onSecurityIndexStateChange(previousState, currentState);
assertEquals(expectedInvalidation, numInvalidation.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
Expand Down Expand Up @@ -138,7 +139,12 @@ private String randomiseDn(String dn) {
}

private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
return indexState(true, indexStatus);
}

private SecurityIndexManager.State indexState(boolean isUpToDate, ClusterHealthStatus healthStatus) {
return new SecurityIndexManager.State(
Instant.now(), isUpToDate, true, true, null, concreteSecurityIndexName, healthStatus, IndexMetaData.State.OPEN);
}

public void testCacheClearOnIndexHealthChange() {
Expand Down Expand Up @@ -172,7 +178,7 @@ public void testCacheClearOnIndexHealthChange() {

// green to yellow or yellow to green
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
store.onSecurityIndexStateChange(previousState, currentState);
assertEquals(expectedInvalidation, numInvalidation.get());
Expand All @@ -182,14 +188,10 @@ public void testCacheClearOnIndexOutOfDateChange() {
final AtomicInteger numInvalidation = new AtomicInteger(0);
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation, true);

store.onSecurityIndexStateChange(
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null),
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null));
store.onSecurityIndexStateChange(indexState(false, null), indexState(true, null));
assertEquals(1, numInvalidation.get());

store.onSecurityIndexStateChange(
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null),
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null));
store.onSecurityIndexStateChange(indexState(true, null), indexState(false, null));
assertEquals(2, numInvalidation.get());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,12 @@ Settings.EMPTY, fileRolesStore, nativeRolesStore, reservedRolesStore, mock(Nativ
}

private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
return new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus);
return dummyIndexState(true, indexStatus);
}

public SecurityIndexManager.State dummyIndexState(boolean isIndexUpToDate, ClusterHealthStatus healthStatus) {
return new SecurityIndexManager.State(
Instant.now(), isIndexUpToDate, true, true, null, concreteSecurityIndexName, healthStatus, IndexMetaData.State.OPEN);
}

public void testCacheClearOnIndexHealthChange() {
Expand Down Expand Up @@ -812,7 +817,7 @@ public void invalidateAll() {

// green to yellow or yellow to green
previousState = dummyState(randomFrom(ClusterHealthStatus.GREEN, ClusterHealthStatus.YELLOW));
currentState = dummyState(previousState.indexStatus == ClusterHealthStatus.GREEN ?
currentState = dummyState(previousState.indexHealth == ClusterHealthStatus.GREEN ?
ClusterHealthStatus.YELLOW : ClusterHealthStatus.GREEN);
compositeRolesStore.onSecurityIndexStateChange(previousState, currentState);
assertEquals(expectedInvalidation, numInvalidation.get());
Expand All @@ -837,14 +842,10 @@ public void invalidateAll() {
}
};

compositeRolesStore.onSecurityIndexStateChange(
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null),
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null));
compositeRolesStore.onSecurityIndexStateChange(dummyIndexState(false, null), dummyIndexState(true, null));
assertEquals(1, numInvalidation.get());

compositeRolesStore.onSecurityIndexStateChange(
new SecurityIndexManager.State(Instant.now(), true, true, true, null, concreteSecurityIndexName, null),
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null));
compositeRolesStore.onSecurityIndexStateChange(dummyIndexState(true, null), dummyIndexState(false, null));
assertEquals(2, numInvalidation.get());
}

Expand Down
Loading

0 comments on commit f32f7d8

Please sign in to comment.