Skip to content

Commit

Permalink
Abstract waitForInit to AbstractSecurityUnitTest and increase configu…
Browse files Browse the repository at this point in the history
…ration load timeout to 10s

Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks committed Jul 19, 2022
1 parent f153c27 commit a110f18
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,13 @@ public Map<CType, SecurityDynamicConfiguration<?>> 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) {
Expand Down
12 changes: 0 additions & 12 deletions src/test/java/org/opensearch/security/RolesInjectorIntegTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,6 @@ public Collection<Object> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,18 +70,6 @@ public Collection<Object> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,18 +66,6 @@ public Collection<Object> 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public static List<AuditMessage> 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);
Expand Down Expand Up @@ -119,7 +119,7 @@ public List<AuditMessage> getFoundMessages() {

private static String createDetailMessage(final int expectedCount, final List<AuditMessage> 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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<Exception> 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,
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/org/opensearch/security/test/SingleClusterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -135,6 +138,31 @@ protected void setupGenericNodes(List<Settings> nodeOverride, List<Boolean> sslO
clusterConfiguration);
}

//Wait for the security plugin to load roles.
public void waitForInit(Client client) throws Exception {
int maxRetries = 3;
Optional<Exception> 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());
}
Expand Down

0 comments on commit a110f18

Please sign in to comment.