From 30ecf8b5e059ce418f667e649258745f4ad8fbd5 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Thu, 24 Jun 2021 11:16:39 +1000 Subject: [PATCH] [Test] Enure test assertions are called for API key creation (#74431) (#74527) Since #74165, API key creation has one additional async layer. Hence the test assertions are now executed asynchronously and may not get called in time before the test ends. This PR ensures the assertions are always called by waiting for a flag. --- .../security/authc/ApiKeyServiceTests.java | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 0da5e72b0281a..c62562f927e26 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.index.IndexAction; +import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.support.PlainActionFuture; @@ -94,6 +95,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; @@ -162,8 +164,7 @@ public void setupMocks() { this.cacheInvalidatorRegistry = mock(CacheInvalidatorRegistry.class); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/74427") - public void testCreateApiKeyWillUseBulkAction() { + public void testCreateApiKeyWillUseBulkAction() throws Exception { final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build(); final ApiKeyService service = createApiKeyService(settings); final Authentication authentication = new Authentication( @@ -173,8 +174,18 @@ public void testCreateApiKeyWillUseBulkAction() { final CreateApiKeyRequest createApiKeyRequest = new CreateApiKeyRequest("key-1", null, null); when(client.prepareIndex(anyString(), anyString())).thenReturn(new IndexRequestBuilder(client, IndexAction.INSTANCE)); when(client.threadPool()).thenReturn(threadPool); + + final AtomicBoolean bulkActionInvoked = new AtomicBoolean(false); + doAnswer(inv -> { + final Object[] args = inv.getArguments(); + BulkRequest bulkRequest = (BulkRequest) args[1]; + assertThat(bulkRequest.numberOfActions(), is(1)); + assertThat(bulkRequest.requests().get(0), instanceOf(IndexRequest.class)); + bulkActionInvoked.set(true); + return null; + }).when(client).execute(eq(BulkAction.INSTANCE), any(BulkRequest.class), any()); service.createApiKey(authentication, createApiKeyRequest, org.elasticsearch.core.Set.of(), new PlainActionFuture<>()); - verify(client).execute(eq(BulkAction.INSTANCE), any(BulkRequest.class), any()); + assertBusy(() -> assertTrue(bulkActionInvoked.get())); } public void testCreateApiKeyWillCacheOnCreation() { @@ -1093,8 +1104,7 @@ public void testApiKeyDocDeserializationWithNullValues() throws IOException { assertEquals(new BytesArray("{}"), apiKeyDoc.roleDescriptorsBytes); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/74427") - public void testCreateApiKeyWillEnsureMetadataCompatibility() { + public void testCreateApiKeyWillEnsureMetadataCompatibility() throws Exception { when(securityIndex.getInstallableMappingVersion()).thenReturn(Version.V_7_12_0); final Settings settings = Settings.builder().put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true).build(); final ApiKeyService service = createApiKeyService(settings); @@ -1110,9 +1120,19 @@ public void testCreateApiKeyWillEnsureMetadataCompatibility() { randomFrom(org.elasticsearch.core.Map.of(), null)); when(client.prepareIndex(anyString(), anyString())).thenReturn(new IndexRequestBuilder(client, IndexAction.INSTANCE)); when(client.threadPool()).thenReturn(threadPool); + + final AtomicBoolean bulkActionInvoked = new AtomicBoolean(false); + doAnswer(inv -> { + final Object[] args = inv.getArguments(); + BulkRequest bulkRequest = (BulkRequest) args[1]; + assertThat(bulkRequest.numberOfActions(), is(1)); + assertThat(bulkRequest.requests().get(0), instanceOf(IndexRequest.class)); + bulkActionInvoked.set(true); + return null; + }).when(client).execute(eq(BulkAction.INSTANCE), any(BulkRequest.class), any()); final PlainActionFuture future2 = new PlainActionFuture<>(); service.createApiKey(authentication, request2, Collections.emptySet(), future2); - verify(client).execute(eq(BulkAction.INSTANCE), any(BulkRequest.class), any()); + assertBusy(() -> assertTrue(bulkActionInvoked.get())); } public void testGetApiKeyMetadata() throws IOException {