Security: Simplify security index listeners (#30466)

This commit adds a general state listener to the SecurityIndexManager,
and replaces the existing health and up-to-date listeners with that. It
also moves helper methods relating to health to SecurityIndexManager
from SecurityLifecycleService.

Author: rjernst

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityLifecycleServiceField.java

@@ -231,7 +231,7 @@
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_FORMAT_SETTING;
 import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED;
-import static org.elasticsearch.xpack.core.security.SecurityLifecycleServiceField.SECURITY_TEMPLATE_NAME;
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME;
 import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY_INDEX_NAME;
 import static org.elasticsearch.xpack.security.support.SecurityIndexManager.INTERNAL_INDEX_FORMAT;
 
@@ -424,8 +424,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
         components.add(realms);
         components.add(reservedRealm);
 
-        securityLifecycleService.securityIndex().addIndexHealthChangeListener(nativeRoleMappingStore::onSecurityIndexHealthChange);
-        securityLifecycleService.securityIndex().addIndexOutOfDateListener(nativeRoleMappingStore::onSecurityIndexOutOfDateChange);
+        securityLifecycleService.securityIndex().addIndexStateListener(nativeRoleMappingStore::onSecurityIndexStateChange);
 
         AuthenticationFailureHandler failureHandler = null;
         String extensionName = null;
@@ -458,8 +457,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
         }
         final CompositeRolesStore allRolesStore = new CompositeRolesStore(settings, fileRolesStore, nativeRolesStore,
             reservedRolesStore, rolesProviders, threadPool.getThreadContext(), getLicenseState());
-        securityLifecycleService.securityIndex().addIndexHealthChangeListener(allRolesStore::onSecurityIndexHealthChange);
-        securityLifecycleService.securityIndex().addIndexOutOfDateListener(allRolesStore::onSecurityIndexOutOfDateChange);
+        securityLifecycleService.securityIndex().addIndexStateListener(allRolesStore::onSecurityIndexStateChange);
         // to keep things simple, just invalidate all cached entries on license change. this happens so rarely that the impact should be
         // minimal
         getLicenseState().addListener(allRolesStore::invalidateAll);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -49,8 +49,6 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
     public static final String INTERNAL_SECURITY_INDEX = SecurityIndexManager.INTERNAL_SECURITY_INDEX;
     public static final String SECURITY_INDEX_NAME = ".security";
 
-    private static final Version MIN_READ_VERSION = Version.V_5_0_0;
-
     private final Settings settings;
     private final ThreadPool threadPool;
     private final IndexAuditTrail indexAuditTrail;
@@ -125,36 +123,7 @@ private void close() {
         }
     }
 
-    public static boolean securityIndexMappingSufficientToRead(ClusterState clusterState, Logger logger) {
-        return checkMappingVersions(clusterState, logger, MIN_READ_VERSION::onOrBefore);
-    }
-
-    static boolean securityIndexMappingUpToDate(ClusterState clusterState, Logger logger) {
-        return checkMappingVersions(clusterState, logger, Version.CURRENT::equals);
-    }
-
-    private static boolean checkMappingVersions(ClusterState clusterState, Logger logger, Predicate<Version> versionPredicate) {
-        return SecurityIndexManager.checkIndexMappingVersionMatches(SECURITY_INDEX_NAME, clusterState, logger, versionPredicate);
-    }
-
     public static List<String> indexNames() {
         return Collections.unmodifiableList(Arrays.asList(SECURITY_INDEX_NAME, INTERNAL_SECURITY_INDEX));
     }
-
-    /**
-     * Is the move from {@code previousHealth} to {@code currentHealth} a move from an unhealthy ("RED") index state to a healthy
-     * ("non-RED") state.
-     */
-    public static boolean isMoveFromRedToNonRed(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
-        return (previousHealth == null || previousHealth.getStatus() == ClusterHealthStatus.RED)
-                && currentHealth != null && currentHealth.getStatus() != ClusterHealthStatus.RED;
-    }
-
-    /**
-     * Is the move from {@code previousHealth} to {@code currentHealth} a move from index-exists to index-deleted
-     */
-    public static boolean isIndexDeleted(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
-        return previousHealth != null && currentHealth == null;
-    }
-
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java

@@ -96,7 +96,7 @@ public static Map<String, Realm.Factory> getFactories(ThreadPool threadPool, Res
         map.put(FileRealmSettings.TYPE, config -> new FileRealm(config, resourceWatcherService));
         map.put(NativeRealmSettings.TYPE, config -> {
             final NativeRealm nativeRealm = new NativeRealm(config, nativeUsersStore);
-            securityLifecycleService.securityIndex().addIndexHealthChangeListener(nativeRealm::onSecurityIndexHealthChange);
+            securityLifecycleService.securityIndex().addIndexStateListener(nativeRealm::onSecurityIndexStateChange);
             return nativeRealm;
         });
         map.put(LdapRealmSettings.AD_TYPE, config -> new LdapRealm(LdapRealmSettings.AD_TYPE, config, sslService,

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/InternalRealms.java

@@ -6,14 +6,16 @@
 package org.elasticsearch.xpack.security.authc.esnative;
 
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.cluster.health.ClusterHealthStatus;
-import org.elasticsearch.cluster.health.ClusterIndexHealth;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
 import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
 import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.security.authc.support.CachingUsernamePasswordRealm;
+import org.elasticsearch.xpack.security.support.SecurityIndexManager;
+
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed;
 
 /**
  * User/password realm that is backed by an Elasticsearch index
@@ -37,12 +39,8 @@ protected void doAuthenticate(UsernamePasswordToken token, ActionListener<Authen
         userStore.verifyPassword(token.principal(), token.credentials(), listener);
     }
 
-    public void onSecurityIndexHealthChange(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
-        final boolean movedFromRedToNonRed = (previousHealth == null || previousHealth.getStatus() == ClusterHealthStatus.RED)
-                && currentHealth != null && currentHealth.getStatus() != ClusterHealthStatus.RED;
-        final boolean indexDeleted = previousHealth != null && currentHealth == null;
-
-        if (movedFromRedToNonRed || indexDeleted) {
+    public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
+        if (isMoveFromRedToNonRed(previousState, currentState) || isIndexDeleted(previousState, currentState)) {
             clearCache();
         }
     }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealm.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.support.ContextPreservingActionListener;
 import org.elasticsearch.client.Client;
+import org.elasticsearch.cluster.health.ClusterHealthStatus;
 import org.elasticsearch.cluster.health.ClusterIndexHealth;
 import org.elasticsearch.common.CheckedBiConsumer;
 import org.elasticsearch.common.bytes.BytesReference;
@@ -38,6 +39,7 @@
 import org.elasticsearch.xpack.security.SecurityLifecycleService;
 import org.elasticsearch.xpack.security.authc.support.CachingUsernamePasswordRealm;
 import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
+import org.elasticsearch.xpack.security.support.SecurityIndexManager;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -61,8 +63,8 @@
 import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
 import static org.elasticsearch.xpack.core.ClientHelper.stashWithOrigin;
 import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY_INDEX_NAME;
-import static org.elasticsearch.xpack.security.SecurityLifecycleService.isIndexDeleted;
-import static org.elasticsearch.xpack.security.SecurityLifecycleService.isMoveFromRedToNonRed;
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed;
 
 /**
  * This store reads + writes {@link ExpressionRoleMapping role mappings} in an Elasticsearch
@@ -316,17 +318,13 @@ private void reportStats(ActionListener<Map<String, Object>> listener, List<Expr
         listener.onResponse(usageStats);
     }
 
-    public void onSecurityIndexHealthChange(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
-        if (isMoveFromRedToNonRed(previousHealth, currentHealth) || isIndexDeleted(previousHealth, currentHealth)) {
+    public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
+        if (isMoveFromRedToNonRed(previousState, currentState) || isIndexDeleted(previousState, currentState) ||
+            previousState.isIndexUpToDate != currentState.isIndexUpToDate) {
             refreshRealms(NO_OP_ACTION_LISTENER, null);
         }
     }
 
-    public void onSecurityIndexOutOfDateChange(boolean prevOutOfDate, boolean outOfDate) {
-        assert prevOutOfDate != outOfDate : "this method should only be called if the two values are different";
-        refreshRealms(NO_OP_ACTION_LISTENER, null);
-    }
-
     private <Result> void refreshRealms(ActionListener<Result> listener, Result result) {
         String[] realmNames = this.realmsToRefresh.toArray(new String[realmsToRefresh.size()]);
         final SecurityClient securityClient = new SecurityClient(client);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java

@@ -7,6 +7,7 @@
 
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.cluster.health.ClusterHealthStatus;
 import org.elasticsearch.cluster.health.ClusterIndexHealth;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
@@ -34,6 +35,7 @@
 import org.elasticsearch.xpack.core.security.authz.privilege.Privilege;
 import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
 import org.elasticsearch.xpack.security.SecurityLifecycleService;
+import org.elasticsearch.xpack.security.support.SecurityIndexManager;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -53,8 +55,8 @@
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.xpack.core.security.SecurityField.setting;
-import static org.elasticsearch.xpack.security.SecurityLifecycleService.isIndexDeleted;
-import static org.elasticsearch.xpack.security.SecurityLifecycleService.isMoveFromRedToNonRed;
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
+import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed;
 
 /**
  * A composite roles store that combines built in roles, file-based roles, and index-based roles. Checks the built in roles first, then the
@@ -323,17 +325,13 @@ public void usageStats(ActionListener<Map<String, Object>> listener) {
         }, listener::onFailure));
     }
 
-    public void onSecurityIndexHealthChange(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
-        if (isMoveFromRedToNonRed(previousHealth, currentHealth) || isIndexDeleted(previousHealth, currentHealth)) {
+    public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
+        if (isMoveFromRedToNonRed(previousState, currentState) || isIndexDeleted(previousState, currentState) ||
+            previousState.isIndexUpToDate != currentState.isIndexUpToDate) {
             invalidateAll();
         }
     }
 
-    public void onSecurityIndexOutOfDateChange(boolean prevOutOfDate, boolean outOfDate) {
-        assert prevOutOfDate != outOfDate : "this method should only be called if the two values are different";
-        invalidateAll();
-    }
-
     /**
      * A mutable class that can be used to represent the combination of one or more {@link IndicesPrivileges}
      */