Wait until security index is ready for role mappings

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.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 @@ private void writeJSONFile(String node, String json) throws Exception {
         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 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("everyone_kibana")) {
+                    if (handlerMetadata != null && handlerMetadata.keys().contains(expectedKey)) {
                         clusterService.removeListener(this);
                         metadataVersion.set(event.state().metadata().version());
                         savedClusterState.countDown();
@@ -280,7 +279,7 @@ private void assertRoleMappingsSaveOK(CountDownLatch savedClusterState, AtomicLo
     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 @@ Collection<Object> createComponents(
         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 @@
 
 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 TransformState transform(Object source, TransformState prevState) throws
         // 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 List<ExpressionRoleMapping> fromXContent(XContentParser parser) throws IO
 
         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 @@
  * 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);
+    }
+}