Wait until security index is ready for role mappings (#92173)

docs/changelog/92173.yaml

@@ -0,0 +1,6 @@
+pr: 92173
+summary: In file based settings, wait until security index is ready for role mappings
+area: Infra/Core
+type: bug
+issues:
+ - 91939

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java

@@ -35,7 +35,6 @@ import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAc
 import org.junit.After;
 
 import java.io.ByteArrayInputStream;
-import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -138,7 +137,7 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase {
         }""";
 
     @After
-    public void cleanUp() throws IOException {
+    public void cleanUp() {
         ClusterUpdateSettingsResponse settingsResponse = client().admin()
             .cluster()
             .prepareUpdateSettings()
@@ -164,7 +163,7 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase {
         Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE);
     }
 
-    private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node) {
+    private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node, String expectedKey) {
         ClusterService clusterService = internalCluster().clusterService(node);
         CountDownLatch savedClusterState = new CountDownLatch(1);
         AtomicLong metadataVersion = new AtomicLong(-1);
@@ -174,7 +173,7 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase {
                 ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
                 if (reservedState != null) {
                     ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedRoleMappingAction.NAME);
-                    if (handlerMetadata != null && handlerMetadata.keys().contains("everyone_kibana")) {
+                    if (handlerMetadata != null && handlerMetadata.keys().contains(expectedKey)) {
                         clusterService.removeListener(this);
                         metadataVersion.set(event.state().metadata().version());
                         savedClusterState.countDown();
@@ -280,7 +279,7 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase {
     public void testRoleMappingsApplied() throws Exception {
         ensureGreen();
 
-        var savedClusterState = setupClusterStateListener(internalCluster().getMasterName());
+        var savedClusterState = setupClusterStateListener(internalCluster().getMasterName(), "everyone_kibana");
         writeJSONFile(internalCluster().getMasterName(), testJSON);
 
         assertRoleMappingsSaveOK(savedClusterState.v1(), savedClusterState.v2());

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsRestartIT.java

@@ -0,0 +1,150 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security;
+
+import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest;
+import org.elasticsearch.cluster.ClusterChangedEvent;
+import org.elasticsearch.cluster.ClusterStateListener;
+import org.elasticsearch.cluster.metadata.ReservedStateHandlerMetadata;
+import org.elasticsearch.cluster.metadata.ReservedStateMetadata;
+import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.core.Strings;
+import org.elasticsearch.core.Tuple;
+import org.elasticsearch.reservedstate.service.FileSettingsService;
+import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.SecurityIntegTestCase;
+import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction;
+import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest;
+import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.Collectors;
+
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.notNullValue;
+
+@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false)
+public class FileSettingsRoleMappingsRestartIT extends SecurityIntegTestCase {
+    private static AtomicLong versionCounter = new AtomicLong(1);
+
+    private static String testJSONOnlyRoleMappings = """
+        {
+             "metadata": {
+                 "version": "%s",
+                 "compatibility": "8.4.0"
+             },
+             "state": {
+                 "role_mappings": {
+                       "everyone_kibana_alone": {
+                          "enabled": true,
+                          "roles": [ "kibana_user" ],
+                          "rules": { "field": { "username": "*" } },
+                          "metadata": {
+                             "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7",
+                             "_foo": "something"
+                          }
+                       },
+                       "everyone_fleet_alone": {
+                          "enabled": true,
+                          "roles": [ "fleet_user" ],
+                          "rules": { "field": { "username": "*" } },
+                          "metadata": {
+                             "uuid" : "b9a59ba9-6b92-4be3-bb8d-02bb270cb3a7",
+                             "_foo": "something_else"
+                          }
+                       }
+                 }
+             }
+        }""";
+
+    private void writeJSONFile(String node, String json) throws Exception {
+        long version = versionCounter.incrementAndGet();
+
+        FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node);
+
+        Files.deleteIfExists(fileSettingsService.operatorSettingsFile());
+
+        Files.createDirectories(fileSettingsService.operatorSettingsDir());
+        Path tempFilePath = createTempFile();
+
+        logger.info("--> writing JSON config to node {} with path {}", node, tempFilePath);
+        logger.info(Strings.format(json, version));
+        Files.write(tempFilePath, Strings.format(json, version).getBytes(StandardCharsets.UTF_8));
+        Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE);
+    }
+
+    private Tuple<CountDownLatch, AtomicLong> setupClusterStateListener(String node, String expectedKey) {
+        ClusterService clusterService = internalCluster().clusterService(node);
+        CountDownLatch savedClusterState = new CountDownLatch(1);
+        AtomicLong metadataVersion = new AtomicLong(-1);
+        clusterService.addListener(new ClusterStateListener() {
+            @Override
+            public void clusterChanged(ClusterChangedEvent event) {
+                ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
+                if (reservedState != null) {
+                    ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedRoleMappingAction.NAME);
+                    if (handlerMetadata != null && handlerMetadata.keys().contains(expectedKey)) {
+                        clusterService.removeListener(this);
+                        metadataVersion.set(event.state().metadata().version());
+                        savedClusterState.countDown();
+                    }
+                }
+            }
+        });
+
+        return new Tuple<>(savedClusterState, metadataVersion);
+    }
+
+    public void testReservedStatePersistsOnRestart() throws Exception {
+        internalCluster().setBootstrapMasterNodeIndex(0);
+
+        final String masterNode = internalCluster().getMasterName();
+        var savedClusterState = setupClusterStateListener(masterNode, "everyone_kibana_alone");
+
+        FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);
+
+        assertTrue(masterFileSettingsService.watching());
+
+        logger.info("--> write some role mappings, no other file settings");
+        writeJSONFile(masterNode, testJSONOnlyRoleMappings);
+        boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS);
+        assertTrue(awaitSuccessful);
+
+        logger.info("--> restart master");
+        internalCluster().restartNode(masterNode);
+
+        var clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet();
+        assertThat(
+            clusterStateResponse.getState()
+                .metadata()
+                .reservedStateMetadata()
+                .get(FileSettingsService.NAMESPACE)
+                .handlers()
+                .get(ReservedRoleMappingAction.NAME)
+                .keys(),
+            containsInAnyOrder("everyone_fleet_alone", "everyone_kibana_alone")
+        );
+
+        var request = new GetRoleMappingsRequest();
+        request.setNames("everyone_kibana_alone", "everyone_fleet_alone");
+        var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get();
+        assertTrue(response.hasMappings());
+        assertThat(
+            Arrays.stream(response.mappings()).map(r -> r.getName()).collect(Collectors.toSet()),
+            allOf(notNullValue(), containsInAnyOrder("everyone_kibana_alone", "everyone_fleet_alone"))
+        );
+    }
+}

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security;
+
+import org.elasticsearch.analysis.common.CommonAnalysisPlugin;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.core.Strings;
+import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.reindex.ReindexPlugin;
+import org.elasticsearch.reservedstate.service.FileSettingsService;
+import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.InternalSettingsPlugin;
+import org.elasticsearch.transport.netty4.Netty4Plugin;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.concurrent.atomic.AtomicLong;
+
+import static org.elasticsearch.test.NodeRoles.dataOnlyNode;
+
+@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false)
+public class FileSettingsRoleMappingsStartupIT extends ESIntegTestCase {
+    private static AtomicLong versionCounter = new AtomicLong(1);
+    private static String testJSONForFailedCase = """
+        {
+             "metadata": {
+                 "version": "%s",
+                 "compatibility": "8.4.0"
+             },
+             "state": {
+                 "role_mappings": {
+                       "everyone_kibana_2": {
+                          "enabled": true,
+                          "roles": [ "kibana_user" ],
+                          "rules": { "field": { "username": "*" } },
+                          "metadata": {
+                             "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7",
+                             "_foo": "something"
+                          }
+                       }
+                 }
+             }
+        }""";
+
+    private void writeJSONFile(String node, String json) throws Exception {
+        long version = versionCounter.incrementAndGet();
+
+        FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node);
+
+        Files.deleteIfExists(fileSettingsService.operatorSettingsFile());
+
+        Files.createDirectories(fileSettingsService.operatorSettingsDir());
+        Path tempFilePath = createTempFile();
+
+        logger.info("--> writing JSON config to node {} with path {}", node, tempFilePath);
+        logger.info(Strings.format(json, version));
+        Files.write(tempFilePath, Strings.format(json, version).getBytes(StandardCharsets.UTF_8));
+        Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE);
+    }
+
+    public void testFailsOnStartMasterNodeWithError() throws Exception {
+        internalCluster().setBootstrapMasterNodeIndex(0);
+
+        String dataNode = internalCluster().startNode(Settings.builder().put(dataOnlyNode()).put("discovery.initial_state_timeout", "1s"));
+        logger.info("--> write some role mappings, no other file settings");
+        writeJSONFile(dataNode, testJSONForFailedCase);
+
+        logger.info("--> stop data node");
+        internalCluster().stopNode(dataNode);
+        logger.info("--> start master node");
+        assertEquals(
+            "unable to launch a new watch service",
+            expectThrows(IllegalStateException.class, () -> internalCluster().startMasterOnlyNode()).getMessage()
+        );
+    }
+
+    public Collection<Class<? extends Plugin>> nodePlugins() {
+        return Arrays.asList(
+            UnstableLocalStateSecurity.class,
+            Netty4Plugin.class,
+            ReindexPlugin.class,
+            CommonAnalysisPlugin.class,
+            InternalSettingsPlugin.class,
+            MapperExtrasPlugin.class
+        );
+    }
+
+    @Override
+    protected boolean addMockTransportService() {
+        return false; // security has its own transport service
+    }
+}

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

@@ -937,6 +937,7 @@ public class Security extends Plugin
         components.add(new SecurityUsageServices(realms, allRolesStore, nativeRoleMappingStore, ipFilter.get(), profileService));
 
         reservedRoleMappingAction.set(new ReservedRoleMappingAction(nativeRoleMappingStore));
+        systemIndices.getMainIndexManager().onStateRecovered(state -> reservedRoleMappingAction.get().securityIndexRecovered());
 
         cacheInvalidatorRegistry.validate();
 

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java

@@ -9,6 +9,7 @@ package org.elasticsearch.xpack.security.action.rolemapping;
 
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.GroupedActionListener;
+import org.elasticsearch.common.util.concurrent.ListenableFuture;
 import org.elasticsearch.reservedstate.NonStateTransformResult;
 import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
 import org.elasticsearch.reservedstate.TransformState;
@@ -41,6 +42,7 @@ public class ReservedRoleMappingAction implements ReservedClusterStateHandler<Li
     public static final String NAME = "role_mappings";
 
     private final NativeRoleMappingStore roleMappingStore;
+    private final ListenableFuture<Void> securityIndexRecoveryListener = new ListenableFuture<>();
 
     /**
      * Creates a ReservedRoleMappingAction
@@ -84,10 +86,17 @@ public class ReservedRoleMappingAction implements ReservedClusterStateHandler<Li
         // non cluster state transform call.
         @SuppressWarnings("unchecked")
         var requests = prepare((List<ExpressionRoleMapping>) source);
-        return new TransformState(prevState.state(), prevState.keys(), l -> nonStateTransform(requests, prevState, l));
+        return new TransformState(
+            prevState.state(),
+            prevState.keys(),
+            l -> securityIndexRecoveryListener.addListener(
+                ActionListener.wrap(ignored -> nonStateTransform(requests, prevState, l), l::onFailure)
+            )
+        );
     }
 
-    private void nonStateTransform(
+    // Exposed for testing purposes
+    protected void nonStateTransform(
         Collection<PutRoleMappingRequest> requests,
         TransformState prevState,
         ActionListener<NonStateTransformResult> listener
@@ -144,4 +153,8 @@ public class ReservedRoleMappingAction implements ReservedClusterStateHandler<Li
 
         return result;
     }
+
+    public void securityIndexRecovered() {
+        securityIndexRecoveryListener.onResponse(null);
+    }
 }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedSecurityStateHandlerProvider.java

@@ -20,7 +20,7 @@ import java.util.Objects;
  * for {@link org.elasticsearch.test.ESIntegTestCase} because the Security Plugin is really LocalStateSecurity in those tests.
  */
 public class LocalReservedSecurityStateHandlerProvider implements ReservedClusterStateHandlerProvider {
-    private final LocalStateSecurity plugin;
+    protected final LocalStateSecurity plugin;
 
     public LocalReservedSecurityStateHandlerProvider() {
         throw new IllegalStateException("Provider must be constructed using PluginsService");

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedUnstableSecurityStateHandlerProvider.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security;
+
+import org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider;
+
+/**
+ * Mock Security Provider implementation for the {@link ReservedClusterStateHandlerProvider} service interface. This is used
+ * for {@link org.elasticsearch.test.ESIntegTestCase} because the Security Plugin is really LocalStateSecurity in those tests.
+ * <p>
+ * Unlike {@link LocalReservedSecurityStateHandlerProvider} this implementation is mocked to implement the
+ * {@link UnstableLocalStateSecurity}. Separate implementation is needed, because the SPI creation code matches the constructor
+ * signature when instantiating. E.g. we need to match {@link UnstableLocalStateSecurity} instead of {@link LocalStateSecurity}
+ */
+public class LocalReservedUnstableSecurityStateHandlerProvider extends LocalReservedSecurityStateHandlerProvider {
+    public LocalReservedUnstableSecurityStateHandlerProvider() {
+        throw new IllegalStateException("Provider must be constructed using PluginsService");
+    }
+
+    public LocalReservedUnstableSecurityStateHandlerProvider(UnstableLocalStateSecurity plugin) {
+        super(plugin);
+    }
+}

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/UnstableLocalStateSecurity.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security;
+
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.license.XPackLicenseState;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.reservedstate.NonStateTransformResult;
+import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
+import org.elasticsearch.reservedstate.TransformState;
+import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest;
+import org.elasticsearch.xpack.core.ssl.SSLService;
+import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction;
+
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * A test class that allows us to Inject new type of Reserved Handler that can
+ * simulate errors in saving role mappings.
+ * <p>
+ * We can't use our regular path to simply make an extension of LocalStateSecurity
+ * in an integration test class, because the reserved handlers are injected through
+ * SPI. (see {@link LocalReservedUnstableSecurityStateHandlerProvider})
+ */
+public class UnstableLocalStateSecurity extends LocalStateSecurity {
+
+    public UnstableLocalStateSecurity(Settings settings, Path configPath) throws Exception {
+        super(settings, configPath);
+        // We reuse most of the initialization of LocalStateSecurity, we then just overwrite
+        // the security plugin with an extra method to give us a fake RoleMappingAction.
+        Optional<Plugin> security = plugins.stream().filter(p -> p instanceof Security).findFirst();
+        if (security.isPresent()) {
+            plugins.remove(security.get());
+        }
+
+        UnstableLocalStateSecurity thisVar = this;
+        var action = new ReservedUnstableRoleMappingAction();
+
+        plugins.add(new Security(settings, super.securityExtensions()) {
+            @Override
+            protected SSLService getSslService() {
+                return thisVar.getSslService();
+            }
+
+            @Override
+            protected XPackLicenseState getLicenseState() {
+                return thisVar.getLicenseState();
+            }
+
+            @Override
+            List<ReservedClusterStateHandler<?>> reservedClusterStateHandlers() {
+                // pretend the security index is initialized after 2 seconds
+                var timer = new java.util.Timer();
+                timer.schedule(new java.util.TimerTask() {
+                    @Override
+                    public void run() {
+                        action.securityIndexRecovered();
+                        timer.cancel();
+                    }
+                }, 2_000);
+                return List.of(action);
+            }
+        });
+    }
+
+    public static class ReservedUnstableRoleMappingAction extends ReservedRoleMappingAction {
+        /**
+         * Creates a fake ReservedRoleMappingAction that doesn't actually use the role mapping store
+         */
+        public ReservedUnstableRoleMappingAction() {
+            // we don't actually need a NativeRoleMappingStore
+            super(null);
+        }
+
+        /**
+         * The nonStateTransform method is the only one that uses the native store, we simply pretend
+         * something has called the onFailure method of the listener.
+         */
+        @Override
+        protected void nonStateTransform(
+            Collection<PutRoleMappingRequest> requests,
+            TransformState prevState,
+            ActionListener<NonStateTransformResult> listener
+        ) {
+            listener.onFailure(new IllegalStateException("Fake exception"));
+        }
+    }
+}

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/reservedstate/ReservedRoleMappingActionTests.java

@@ -76,6 +76,7 @@ public class ReservedRoleMappingActionTests extends ESTestCase {
         ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build();
         TransformState prevState = new TransformState(state, Collections.emptySet());
         ReservedRoleMappingAction action = new ReservedRoleMappingAction(nativeRoleMappingStore);
+        action.securityIndexRecovered();
 
         String badPolicyJSON = """
             {
@@ -109,6 +110,7 @@ public class ReservedRoleMappingActionTests extends ESTestCase {
         ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build();
         TransformState prevState = new TransformState(state, Collections.emptySet());
         ReservedRoleMappingAction action = new ReservedRoleMappingAction(nativeRoleMappingStore);
+        action.securityIndexRecovered();
 
         String emptyJSON = "";
 
@@ -181,6 +183,7 @@ public class ReservedRoleMappingActionTests extends ESTestCase {
         ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build();
         TransformState updatedState = new TransformState(state, Collections.emptySet());
         ReservedRoleMappingAction action = new ReservedRoleMappingAction(nativeRoleMappingStore);
+        action.securityIndexRecovered();
 
         String json = """
             {

x-pack/plugin/security/src/test/resources/META-INF/services/org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider

@@ -6,3 +6,4 @@
 #
 
 org.elasticsearch.xpack.security.LocalReservedSecurityStateHandlerProvider
+org.elasticsearch.xpack.security.LocalReservedUnstableSecurityStateHandlerProvider