From a110f184025ca1a07e7a64d6444b8fea2f73039b Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 19 Jul 2022 10:58:34 -0400 Subject: [PATCH] Abstract waitForInit to AbstractSecurityUnitTest and increase configuration load timeout to 10s Signed-off-by: Craig Perkins --- .../ConfigurationRepository.java | 4 +-- .../security/RolesInjectorIntegTest.java | 12 ------- .../security/RolesValidationIntegTest.java | 13 -------- .../TransportUserInjectorIntegTest.java | 13 -------- .../integration/TestAuditlogImpl.java | 4 +-- .../ccstest/CrossClusterSearchTests.java | 22 ++----------- .../dlic/dlsfls/CCReplicationTest.java | 12 ++++--- .../dlic/rest/api/TenantInfoActionTest.java | 5 +++ .../test/AbstractSecurityUnitTest.java | 32 ++++++++++++++++++- .../security/test/SingleClusterTest.java | 28 ++++++++++++++++ 10 files changed, 78 insertions(+), 67 deletions(-) diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java index 84d3059942..81f5c5d60d 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -363,13 +363,13 @@ public Map> getConfigurationsFromIndex(Co } else { LOGGER.debug("security index exists and was created with ES 7 (new layout)"); } - retVal.putAll(validate(cl.load(configTypes.toArray(new CType[0]), 5, TimeUnit.SECONDS, acceptInvalid), configTypes.size())); + retVal.putAll(validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size())); } else { //wait (and use new layout) LOGGER.debug("security index not exists (yet)"); - retVal.putAll(validate(cl.load(configTypes.toArray(new CType[0]), 5, TimeUnit.SECONDS, acceptInvalid), configTypes.size())); + retVal.putAll(validate(cl.load(configTypes.toArray(new CType[0]), 10, TimeUnit.SECONDS, acceptInvalid), configTypes.size())); } } catch (Exception e) { diff --git a/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java b/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java index aac9a93b98..9a356ff92e 100644 --- a/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java +++ b/src/test/java/org/opensearch/security/RolesInjectorIntegTest.java @@ -74,18 +74,6 @@ public Collection createComponents(Client client, ClusterService cluster } } - //Wait for the security plugin to load roles. - private void waitForInit(Client client) throws Exception { - try { - client.admin().cluster().health(new ClusterHealthRequest()).actionGet(); - } catch (OpenSearchSecurityException ex) { - if(ex.getMessage().contains("OpenSearch Security not initialized")) { - Thread.sleep(500); - waitForInit(client); - } - } - } - @Test public void testRolesInject() throws Exception { setup(Settings.EMPTY, new DynamicSecurityConfig().setSecurityRoles("roles.yml"), Settings.EMPTY); diff --git a/src/test/java/org/opensearch/security/RolesValidationIntegTest.java b/src/test/java/org/opensearch/security/RolesValidationIntegTest.java index 86168c0c14..588bcbb7fc 100644 --- a/src/test/java/org/opensearch/security/RolesValidationIntegTest.java +++ b/src/test/java/org/opensearch/security/RolesValidationIntegTest.java @@ -20,7 +20,6 @@ import org.junit.Test; import org.opensearch.OpenSearchSecurityException; -import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; import org.opensearch.action.admin.indices.exists.indices.IndicesExistsRequest; @@ -71,18 +70,6 @@ public Collection createComponents(Client client, ClusterService cluster } } - //Wait for the security plugin to load roles. - private void waitForInit(Client client) throws Exception { - try { - client.admin().cluster().health(new ClusterHealthRequest()).actionGet(); - } catch (OpenSearchSecurityException ex) { - if(ex.getMessage().contains("OpenSearch Security not initialized")) { - Thread.sleep(500); - waitForInit(client); - } - } - } - @Test public void testRolesValidation() throws Exception { setup(Settings.EMPTY, new DynamicSecurityConfig().setSecurityRoles("roles.yml"), Settings.EMPTY); diff --git a/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java b/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java index 065b99146b..56a7b727b4 100644 --- a/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java +++ b/src/test/java/org/opensearch/security/TransportUserInjectorIntegTest.java @@ -20,7 +20,6 @@ import org.junit.Test; import org.opensearch.OpenSearchSecurityException; -import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; import org.opensearch.client.Client; @@ -67,18 +66,6 @@ public Collection createComponents(Client client, ClusterService cluster } } - //Wait for the security plugin to load roles. - private void waitForInit(Client client) throws Exception { - try { - client.admin().cluster().health(new ClusterHealthRequest()).actionGet(); - } catch (OpenSearchSecurityException ex) { - if(ex.getMessage().contains("OpenSearch Security not initialized")) { - Thread.sleep(500); - waitForInit(client); - } - } - } - @Test public void testSecurityUserInjection() throws Exception { final Settings clusterNodeSettings = Settings.builder() diff --git a/src/test/java/org/opensearch/security/auditlog/integration/TestAuditlogImpl.java b/src/test/java/org/opensearch/security/auditlog/integration/TestAuditlogImpl.java index 5c23f1b37c..4677bc37a9 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/TestAuditlogImpl.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/TestAuditlogImpl.java @@ -74,7 +74,7 @@ public static List doThenWaitForMessages(final Runnable action, fi throw new MessagesNotFoundException(expectedCount, messages); } if (messages.size() != expectedCount) { - throw new RuntimeException("Unexpected number of messages, was expecting " + expectedCount + ", recieved " + messages.size()); + throw new RuntimeException("Unexpected number of messages, was expecting " + expectedCount + ", received " + messages.size()); } } catch (final InterruptedException e) { throw new RuntimeException("Unexpected exception", e); @@ -119,7 +119,7 @@ public List getFoundMessages() { private static String createDetailMessage(final int expectedCount, final List foundMessages) { return new StringBuilder() - .append("Did not recieve all " + expectedCount + " audit messages after a short wait. ") + .append("Did not receive all " + expectedCount + " audit messages after a short wait. ") .append("Missing " + (expectedCount - foundMessages.size()) + " messages.") .append("Messages found during this time: \n\n") .append(foundMessages.stream() diff --git a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java index 246e159c5a..e6b533c0b9 100644 --- a/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java +++ b/src/test/java/org/opensearch/security/ccstest/CrossClusterSearchTests.java @@ -982,24 +982,6 @@ public void testCcsWithDiffCertsWithNodesDnDynamicallyAdded() throws Exception { assertThat(ccs.getBody(), containsString("cross_cluster_two:twitter")); } - //Wait for the security plugin to load roles. - private void waitOrThrow(Client client) throws Exception { - int failures = 0; - while(failures < 5) { - try { - client.admin().cluster().health(new ClusterHealthRequest()).actionGet(); - break; - } catch (OpenSearchSecurityException ex) { - if (ex.getMessage().contains("OpenSearch Security not initialized")) { - Thread.sleep(500); - failures++; - } else { - throw ex; - } - } - } - } - @Test public void testCcsWithRoleInjection() throws Exception { setupCcs(new DynamicSecurityConfig().setSecurityRoles("roles.yml")); @@ -1041,7 +1023,7 @@ public void testCcsWithRoleInjection() throws Exception { RolesInjectorIntegTest.RolesInjectorPlugin.injectedRoles = "invalid_user|invalid_role"; try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorIntegTest.RolesInjectorPlugin.class).start()) { - waitOrThrow(node.client()); + waitForInit(node.client()); Client remoteClient = node.client().getRemoteClusterClient("cross_cluster_two"); GetRequest getReq = new GetRequest("twitter", "0"); getReq.realtime(true); @@ -1061,7 +1043,7 @@ public void testCcsWithRoleInjection() throws Exception { RolesInjectorIntegTest.RolesInjectorPlugin.injectedRoles = "valid_user|opendistro_security_all_access"; try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, RolesInjectorIntegTest.RolesInjectorPlugin.class).start()) { - waitOrThrow(node.client()); + waitForInit(node.client()); Client remoteClient = node.client().getRemoteClusterClient("cross_cluster_two"); GetRequest getReq = new GetRequest("twitter", "0"); getReq.realtime(true); diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java index 8ceb4c336c..2e582daf88 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/CCReplicationTest.java @@ -212,7 +212,8 @@ public void testReplication() throws Exception { // Set roles for the user MockReplicationPlugin.injectedRoles = "ccr_user|opendistro_security_human_resources_trainee"; try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class).start()) { - waitOrThrow(node.client(), "hr-dls"); + waitForInit(node.client()); + node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest("hr-dls")).actionGet(); Assert.fail("Expecting exception"); } catch (OpenSearchSecurityException ex) { log.warn(ex.getMessage()); @@ -222,7 +223,8 @@ public void testReplication() throws Exception { } try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class).start()) { - waitOrThrow(node.client(), "hr-fls"); + waitForInit(node.client()); + node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest("hr-fls")).actionGet(); Assert.fail("Expecting exception"); } catch (OpenSearchSecurityException ex) { log.warn(ex.getMessage()); @@ -232,7 +234,8 @@ public void testReplication() throws Exception { } try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class).start()) { - waitOrThrow(node.client(), "hr-masking"); + waitForInit(node.client()); + node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest("hr-masking")).actionGet(); Assert.fail("Expecting exception"); } catch (OpenSearchSecurityException ex) { log.warn(ex.getMessage()); @@ -242,7 +245,8 @@ public void testReplication() throws Exception { } try (Node node = new PluginAwareNode(false, tcSettings, Netty4Plugin.class, OpenSearchSecurityPlugin.class, MockReplicationPlugin.class).start()) { - waitOrThrow(node.client(), "hr-normal"); + waitForInit(node.client()); + node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest("hr-normal")).actionGet(); AcknowledgedResponse res = node.client().execute(MockReplicationAction.INSTANCE, new MockReplicationRequest("hr-normal")).actionGet(); Assert.assertTrue(res.isAcknowledged()); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/TenantInfoActionTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/TenantInfoActionTest.java index 01004faba7..d647d56a55 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/TenantInfoActionTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/TenantInfoActionTest.java @@ -16,6 +16,7 @@ import org.junit.Assert; import org.junit.Test; +import org.opensearch.client.Client; import org.opensearch.common.settings.Settings; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.helper.rest.RestHelper; @@ -41,6 +42,9 @@ public TenantInfoActionTest(){ public void testTenantInfoAPIAccess() throws Exception { Settings settings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build(); setup(settings); + try (Client tc = getClient()) { + waitForInit(tc); + } rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; @@ -60,6 +64,7 @@ public void testTenantInfoAPIAccess() throws Exception { public void testTenantInfoAPIUpdate() throws Exception { Settings settings = Settings.builder().put(ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION, true).build(); setup(settings); + rh.keystore = "restapi/kirk-keystore.jks"; rh.sendHTTPClientCredentials = true; rh.sendAdminCertificate = true; diff --git a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java index c5c3172855..f9913d9478 100644 --- a/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java +++ b/src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java @@ -34,6 +34,7 @@ import java.util.Base64; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.SSLContext; @@ -57,6 +58,8 @@ import org.junit.rules.TestName; import org.junit.rules.TestWatcher; +import org.opensearch.OpenSearchSecurityException; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.cluster.node.info.NodesInfoRequest; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.index.IndexRequest; @@ -83,7 +86,7 @@ /* * There are real thread leaks during test execution, not all threads are - * properly waited on or interupted. While this normally doesn't create test + * properly waited on or interrupted. While this normally doesn't create test * failures, retries mitigate this. Remove this attribute to explore these * issues. */ @@ -164,6 +167,33 @@ protected RestHighLevelClient getRestClient(ClusterInfo info, String keyStoreNam } } + /** Wait for the security plugin to load roles. */ + public void waitForInit(Client client) { + int maxRetries = 5; + Optional retainedException = Optional.empty(); + for (int i = 0; i < maxRetries; i++) { + try { + client.admin().cluster().health(new ClusterHealthRequest()).actionGet(); + retainedException = Optional.empty(); + return; + } catch (OpenSearchSecurityException ex) { + if(ex.getMessage().contains("OpenSearch Security not initialized")) { + retainedException = Optional.of(ex); + try { + Thread.sleep(500); + } catch (InterruptedException e) { /* ignored */ } + } else { + // plugin is initialized, but another error received. + // Example could be user does not have permissions for cluster:monitor/health + retainedException = Optional.empty(); + } + } + } + if (retainedException.isPresent()) { + throw new RuntimeException(retainedException.get()); + } + } + protected void initialize(ClusterHelper clusterHelper, ClusterInfo clusterInfo, DynamicSecurityConfig securityConfig) throws IOException { try (Client tc = clusterHelper.nodeClient()) { Assert.assertEquals(clusterInfo.numNodes, diff --git a/src/test/java/org/opensearch/security/test/SingleClusterTest.java b/src/test/java/org/opensearch/security/test/SingleClusterTest.java index 0c2f3bfc07..ed1f4e9148 100644 --- a/src/test/java/org/opensearch/security/test/SingleClusterTest.java +++ b/src/test/java/org/opensearch/security/test/SingleClusterTest.java @@ -28,10 +28,13 @@ import java.io.File; import java.util.List; +import java.util.Optional; import org.junit.After; import org.junit.Assert; +import org.opensearch.OpenSearchSecurityException; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.client.Client; import org.opensearch.common.settings.Settings; import org.opensearch.security.test.helper.cluster.ClusterConfiguration; @@ -135,6 +138,31 @@ protected void setupGenericNodes(List nodeOverride, List sslO clusterConfiguration); } + //Wait for the security plugin to load roles. + public void waitForInit(Client client) throws Exception { + int maxRetries = 3; + Optional retainedException = Optional.empty(); + for (int i = 0; i < maxRetries; i++) { + try { + client.admin().cluster().health(new ClusterHealthRequest()).actionGet(); + retainedException = Optional.empty(); + return; + } catch (OpenSearchSecurityException ex) { + if(ex.getMessage().contains("OpenSearch Security not initialized")) { + retainedException = Optional.of(ex); + Thread.sleep(500); + } else { + // plugin is initialized, but another error received. + // Example could be user does not have permissions for cluster:monitor/health + retainedException = Optional.empty(); + } + } + } + if (retainedException.isPresent()) { + throw retainedException.get(); + } + } + protected RestHelper restHelper() { return new RestHelper(clusterInfo, getResourceFolder()); }