Skip to content

Commit

Permalink
Allow SecurityIndexManager to update index mappings (#68729)
Browse files Browse the repository at this point in the history 
While backporting #67114 via #68375, I realised that there are existing
upgrade scenarios that expect the `SecurityIndexManager` to update index
mappings, so in the backport PR, this capability was reinstated. This
commit does the same in `master`.
  • Loading branch information
@pugnascotia
pugnascotia authored Feb 9, 2021
1 parent 8fff763 commit dd2e1d7
Show file tree
Hide file tree
Showing 8 changed files with 248 additions and 97 deletions.
79 changes: 69 additions & 10 deletions ...security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import org.elasticsearch.action.admin.indices.alias.Alias;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.master.AcknowledgedResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.ClusterChangedEvent;
import org.elasticsearch.cluster.ClusterState;
Expand All @@ -33,6 +35,7 @@
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.routing.IndexRoutingTable;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.gateway.GatewayService;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
Expand All @@ -49,6 +52,7 @@
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static org.elasticsearch.cluster.metadata.IndexMetadata.INDEX_FORMAT_SETTING;
Expand Down Expand Up @@ -118,6 +122,10 @@ public boolean isAvailable() {
return this.indexState.indexAvailable;
}

public boolean isMappingUpToDate() {
return this.indexState.mappingUpToDate;
}

public boolean isStateRecovered() {
return this.indexState != State.UNRECOVERED_STATE;
}
Expand Down Expand Up @@ -161,6 +169,7 @@ public void clusterChanged(ClusterChangedEvent event) {
final boolean isIndexUpToDate = indexMetadata == null ||
INDEX_FORMAT_SETTING.get(indexMetadata.getSettings()) == systemIndexDescriptor.getIndexFormat();
final boolean indexAvailable = checkIndexAvailable(event.state());
final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state());
final Version mappingVersion = oldestIndexMappingVersion(event.state());
final String concreteIndexName = indexMetadata == null
? systemIndexDescriptor.getPrimaryIndex()
Expand All @@ -180,8 +189,8 @@ public void clusterChanged(ClusterChangedEvent event) {
final IndexRoutingTable routingTable = event.state().getRoutingTable().index(indexMetadata.getIndex());
indexHealth = new ClusterIndexHealth(indexMetadata, routingTable).getStatus();
}
final State newState = new State(creationTime, isIndexUpToDate, indexAvailable, mappingVersion,
concreteIndexName, indexHealth, indexState);
final State newState = new State(creationTime, isIndexUpToDate, indexAvailable, mappingIsUpToDate, mappingVersion,
concreteIndexName, indexHealth, indexState, event.state().nodes().getMinNodeVersion());
this.indexState = newState;

if (newState.equals(previousState) == false) {
Expand Down Expand Up @@ -211,6 +220,26 @@ private boolean checkIndexAvailable(ClusterState state) {
}
}

private boolean checkIndexMappingUpToDate(ClusterState clusterState) {
/*
* The method reference looks wrong here, but it's just counter-intuitive. It expands to:
*
* mappingVersion -> Version.CURRENT.onOrBefore(mappingVersion)
*
* ...which is true if the mappings have been updated.
*/
return checkIndexMappingVersionMatches(clusterState, Version.CURRENT::onOrBefore);
}

private boolean checkIndexMappingVersionMatches(ClusterState clusterState, Predicate<Version> predicate) {
return checkIndexMappingVersionMatches(this.systemIndexDescriptor.getAliasName(), clusterState, logger, predicate);
}

public static boolean checkIndexMappingVersionMatches(String indexName, ClusterState clusterState, Logger logger,
Predicate<Version> predicate) {
return loadIndexMappingVersions(indexName, clusterState, logger).stream().allMatch(predicate);
}

private Version oldestIndexMappingVersion(ClusterState clusterState) {
final Set<Version> versions = loadIndexMappingVersions(systemIndexDescriptor.getAliasName(), clusterState, logger);
return versions.stream().min(Version::compareTo).orElse(null);
Expand All @@ -220,9 +249,9 @@ private static Set<Version> loadIndexMappingVersions(String aliasName, ClusterSt
Set<Version> versions = new HashSet<>();
IndexMetadata indexMetadata = resolveConcreteIndex(aliasName, clusterState.metadata());
if (indexMetadata != null) {
MappingMetadata mmd = indexMetadata.mapping();
if (mmd != null) {
versions.add(readMappingVersion(aliasName, mmd, logger));
MappingMetadata mappingMetadata = indexMetadata.mapping();
if (mappingMetadata != null) {
versions.add(readMappingVersion(aliasName, mappingMetadata, logger));
}
}
return versions;
Expand Down Expand Up @@ -335,6 +364,29 @@ public void onFailure(Exception e) {
}
}
}, client.admin().indices()::create);
} else if (indexState.mappingUpToDate == false) {
final String error = systemIndexDescriptor.checkMinimumNodeVersion("create index", indexState.minimumNodeVersion);
if (error != null) {
consumer.accept(new IllegalStateException(error));
} else {
logger.info(
"Index [{}] (alias [{}]) is not up to date. Updating mapping",
indexState.concreteIndexName,
systemIndexDescriptor.getAliasName()
);
PutMappingRequest request = new PutMappingRequest(indexState.concreteIndexName).source(
systemIndexDescriptor.getMappings(),
XContentType.JSON
).origin(systemIndexDescriptor.getOrigin());
executeAsyncWithOrigin(client.threadPool().getThreadContext(), systemIndexDescriptor.getOrigin(), request,
ActionListener.<AcknowledgedResponse>wrap(putMappingResponse -> {
if (putMappingResponse.isAcknowledged()) {
andThen.run();
} else {
consumer.accept(new IllegalStateException("put mapping request was not acknowledged"));
}
}, consumer), client.admin().indices()::putMapping);
}
} else {
andThen.run();
}
Expand Down Expand Up @@ -362,24 +414,29 @@ public static boolean isIndexDeleted(State previousState, State currentState) {
* State of the security index.
*/
public static class State {
public static final State UNRECOVERED_STATE = new State(null, false, false, null, null, null, null);
public static final State UNRECOVERED_STATE = new State(null, false, false, false, null, 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 indexHealth;
public final IndexMetadata.State indexState;
public final Version minimumNodeVersion;

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

@Override
Expand All @@ -390,10 +447,12 @@ public boolean equals(Object o) {
return Objects.equals(creationTime, state.creationTime) &&
isIndexUpToDate == state.isIndexUpToDate &&
indexAvailable == state.indexAvailable &&
mappingUpToDate == state.mappingUpToDate &&
Objects.equals(mappingVersion, state.mappingVersion) &&
Objects.equals(concreteIndexName, state.concreteIndexName) &&
indexHealth == state.indexHealth &&
indexState == state.indexState;
indexState == state.indexState &&
Objects.equals(minimumNodeVersion, state.minimumNodeVersion);
}

public boolean indexExists() {
Expand All @@ -402,8 +461,8 @@ public boolean indexExists() {

@Override
public int hashCode() {
return Objects.hash(creationTime, isIndexUpToDate, indexAvailable, mappingVersion, concreteIndexName,
indexHealth);
return Objects.hash(creationTime, isIndexUpToDate, indexAvailable, mappingUpToDate, mappingVersion, concreteIndexName,
indexHealth, minimumNodeVersion);
}
}
}
2 changes: 1 addition & 1 deletion ...rity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,6 @@ private void setCompletedToTrue(AtomicBoolean completed) {

private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {
return new SecurityIndexManager.State(
Instant.now(), true, true, null, concreteSecurityIndexName, indexStatus, IndexMetadata.State.OPEN);
Instant.now(), true, true, true, null, concreteSecurityIndexName, indexStatus, IndexMetadata.State.OPEN, null);
}
}
2 changes: 1 addition & 1 deletion ...urity/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class NativeRealmTests extends ESTestCase {

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

public void testCacheClearOnIndexHealthChange() {
Expand Down
2 changes: 1 addition & 1 deletion ...va/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {

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

public void testCacheClearOnIndexHealthChange() {
Expand Down
2 changes: 1 addition & 1 deletion .../src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) {

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

public void testCacheClearOnIndexHealthChange() {
Expand Down
4 changes: 2 additions & 2 deletions ...src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,8 @@ public void testGetPrivilegesWorkWithoutCache() throws Exception {
private SecurityIndexManager.State dummyState(
String concreteSecurityIndexName, boolean isIndexUpToDate, ClusterHealthStatus healthStatus) {
return new SecurityIndexManager.State(
Instant.now(), isIndexUpToDate, true, null,
concreteSecurityIndexName, healthStatus, IndexMetadata.State.OPEN
Instant.now(), isIndexUpToDate, true, true, null,
concreteSecurityIndexName, healthStatus, IndexMetadata.State.OPEN, null
);
}

Expand Down
4 changes: 2 additions & 2 deletions ...src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public void testSecurityIndexStateChangeWillInvalidateAllRegisteredInvalidators(

final SecurityIndexManager.State previousState = SecurityIndexManager.State.UNRECOVERED_STATE;
final SecurityIndexManager.State currentState = new SecurityIndexManager.State(
Instant.now(), true, true, Version.CURRENT,
".security", ClusterHealthStatus.GREEN, IndexMetadata.State.OPEN);
Instant.now(), true, true, true, Version.CURRENT,
".security", ClusterHealthStatus.GREEN, IndexMetadata.State.OPEN, null);

cacheInvalidatorRegistry.onSecurityIndexStageChange(previousState, currentState);
verify(invalidator1).invalidateAll();
Expand Down
Loading

0 comments on commit dd2e1d7

Please sign in to comment.