Fix assertSecurityIndexActive to actually wait (#112924) (#113372)

One of the test util assertion methods `assertSecurityIndexActive`
currently completes successfully if the security index does not exist or
if it's unavailable, which is not what the method name implies. This PR
fixes the `assertBusy` call to properly wait for the security index.

Author: n1v0lg

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/NativeRealmIntegTestCase.java

@@ -36,7 +36,7 @@ public abstract class NativeRealmIntegTestCase extends SecurityIntegTestCase {
 
     @Before
     public void ensureNativeStoresStarted() throws Exception {
-        assertSecurityIndexActive();
+        createSecurityIndexWithWaitForActiveShards();
         if (shouldSetReservedUserPasswords()) {
             setupReservedPasswords();
         }

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

@@ -191,7 +191,7 @@ protected boolean addMockHttpTransport() {
 
     @Before
     public void waitForSecurityIndexWritable() throws Exception {
-        assertSecurityIndexActive();
+        createSecurityIndexWithWaitForActiveShards();
     }
 
     @After

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

@@ -783,7 +783,7 @@ public void testAuthenticateWithWrongToken() throws Exception {
 
     @Before
     public void waitForSecurityIndexWritable() throws Exception {
-        assertSecurityIndexActive();
+        createSecurityIndexWithWaitForActiveShards();
     }
 
     @After

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

@@ -31,7 +31,7 @@
 public class SecurityScrollTests extends SecurityIntegTestCase {
 
     public void testScrollIsPerUser() throws Exception {
-        assertSecurityIndexActive();
+        createSecurityIndexWithWaitForActiveShards();
         new PutRoleRequestBuilder(client()).name("scrollable")
             .addIndices(new String[] { randomAlphaOfLengthBetween(4, 12) }, new String[] { "read" }, null, null, null, randomBoolean())
             .get();

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

@@ -46,7 +46,6 @@
 public class SecurityIndexManagerIntegTests extends SecurityIntegTestCase {
 
     public void testConcurrentOperationsTryingToCreateSecurityIndexAndAlias() throws Exception {
-        assertSecurityIndexActive();
         final int processors = Runtime.getRuntime().availableProcessors();
         final int numThreads = Math.min(50, scaledRandomIntBetween((processors + 1) / 2, 4 * processors));  // up to 50 threads
         final int maxNumRequests = 50 / numThreads; // bound to a maximum of 50 requests
@@ -111,7 +110,7 @@ public void testOnIndexAvailableForSearchIndexCompletesWithinTimeout() throws Ex
         // pick longer wait than in the assertBusy that waits for below to ensure index has had enough time to initialize
         securityIndexManager.onIndexAvailableForSearch((ActionListener<Void>) future, TimeValue.timeValueSeconds(40));
 
-        createSecurityIndex();
+        createSecurityIndexWithWaitForActiveShards();
 
         assertBusy(
             () -> assertThat(securityIndexManager.isAvailable(SecurityIndexManager.Availability.SEARCH_SHARDS), is(true)),
@@ -126,7 +125,7 @@ public void testOnIndexAvailableForSearchIndexCompletesWithinTimeout() throws Ex
 
     @SuppressWarnings("unchecked")
     public void testOnIndexAvailableForSearchIndexAlreadyAvailable() throws Exception {
-        createSecurityIndex();
+        createSecurityIndexWithWaitForActiveShards();
 
         final SecurityIndexManager securityIndexManager = internalCluster().getInstances(NativePrivilegeStore.class)
             .iterator()

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

@@ -14,6 +14,7 @@
 import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
 import org.elasticsearch.action.admin.indices.get.GetIndexRequest;
 import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
+import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.internal.Client;
@@ -381,6 +382,12 @@ protected Function<Client, Client> getClientWrapper() {
         return client -> (client instanceof NodeClient) ? client.filterWithHeader(headers) : client;
     }
 
+    /**
+     * Waits for security index to become available. Note that you must ensure index creation was triggered before calling this method,
+     * by calling one of the resource creation APIs (e.g., creating a user).
+     * If you use {@link #createSecurityIndexWithWaitForActiveShards()} to create the index it's not necessary to call
+     * {@link #assertSecurityIndexActive} since the create method ensures the index is active.
+     */
     public void assertSecurityIndexActive() throws Exception {
         assertSecurityIndexActive(cluster());
     }
@@ -391,14 +398,10 @@ public void assertSecurityIndexActive(TestCluster testCluster) throws Exception
                 ClusterState clusterState = client.admin().cluster().prepareState(TEST_REQUEST_TIMEOUT).setLocal(true).get().getState();
                 assertFalse(clusterState.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK));
                 Index securityIndex = resolveSecurityIndex(clusterState.metadata());
-                // TODO this is a bug -- since we are not tripping assertions here, this will complete successfully even if the security
-                // index does not exist
-                if (securityIndex != null) {
-                    IndexRoutingTable indexRoutingTable = clusterState.routingTable().index(securityIndex);
-                    if (indexRoutingTable != null) {
-                        assertTrue(indexRoutingTable.allPrimaryShardsActive());
-                    }
-                }
+                assertNotNull(securityIndex);
+                IndexRoutingTable indexRoutingTable = clusterState.routingTable().index(securityIndex);
+                assertNotNull(indexRoutingTable);
+                assertTrue(indexRoutingTable.allPrimaryShardsActive());
             }, 30L, TimeUnit.SECONDS);
         }
     }
@@ -424,7 +427,7 @@ protected void deleteSecurityIndex() {
         }
     }
 
-    protected void createSecurityIndex() {
+    protected void createSecurityIndexWithWaitForActiveShards() {
         final Client client = client().filterWithHeader(
             Collections.singletonMap(
                 "Authorization",
@@ -434,7 +437,8 @@ protected void createSecurityIndex() {
                 )
             )
         );
-        CreateIndexRequest createIndexRequest = new CreateIndexRequest(SECURITY_MAIN_ALIAS);
+        CreateIndexRequest createIndexRequest = new CreateIndexRequest(SECURITY_MAIN_ALIAS).waitForActiveShards(ActiveShardCount.ALL)
+            .masterNodeTimeout(TEST_REQUEST_TIMEOUT);
         client.admin().indices().create(createIndexRequest).actionGet();
     }
 

x-pack/qa/third-party/active-directory/src/test/java/org/elasticsearch/xpack/security/authc/ldap/AbstractAdLdapRealmTestCase.java

@@ -181,7 +181,7 @@ protected Settings buildRealmSettings(
 
     @Before
     public void setupRoleMappings() throws Exception {
-        assertSecurityIndexActive();
+        createSecurityIndexWithWaitForActiveShards();
 
         List<String> content = getRoleMappingContent(RoleMappingEntry::nativeContent);
         if (content.isEmpty()) {