Test: speed up IndexPrivilegeTests (elastic/x-pack-elasticsearch#4348)

The IndexPrivilegeTests have been notoriously slow for years.
@polyfractal identified the primary issue, which is that these tests
were running against an internal cluster with 1 or 2 data nodes and had
the number of replicas set to 1 for indices by default and the methods
in the test would perform a wait for green. This wait for green would
take the full thirty seconds when there was a single data node as the
index could never reach green health due to an unassigned replica. This
could have been caught earlier by asserting the request did not timeout
but this assertion was not present.

This change does a few things to address the issues above. The first is
that these tests now extend SecuritySingleNodeTestCase, which is a new
class that extends ESSingleNodeTestCase and contains the necessary
logic for the setup and teardown of security; much of which is based
off of SecurityIntegTestCase. This means that these tests always run
against a single node cluster and have a much simpler setup. The
default index template for these tests applies settings so that indices
are created with a single shard and no replicas.

Assertions have been added to ensure the health checks with a wait for
green status have not timed out. A handcoded wait for snapshots to
finish has been replaced with an assertBusy call. Finally, the BadApple
annotation has been removed from the test.

Relates elastic/x-pack-elasticsearch#324

Original commit: elastic/x-pack-elasticsearch@572919273d
This commit is contained in:
Jay Modi 2018-04-13 06:30:40 -06:00 committed by GitHub
parent c09c9e13d7
commit 0d83edbca5
5 changed files with 285 additions and 63 deletions

View File

@ -14,7 +14,7 @@ import org.apache.http.util.EntityUtils;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.test.SecurityIntegTestCase;
import org.elasticsearch.test.SecuritySingleNodeTestCase;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
@ -31,7 +31,7 @@ import static org.hamcrest.Matchers.not;
/**
* a helper class that contains a couple of HTTP helper methods
*/
public abstract class AbstractPrivilegeTestCase extends SecurityIntegTestCase {
public abstract class AbstractPrivilegeTestCase extends SecuritySingleNodeTestCase {
protected static final String USERS_PASSWD_HASHED = new String(Hasher.BCRYPT.hash(new SecureString("passwd".toCharArray())));

View File

@ -5,13 +5,11 @@
*/
package org.elasticsearch.integration;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.cluster.snapshots.status.SnapshotsStatusResponse;
import org.elasticsearch.cluster.SnapshotsInProgress;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
import org.elasticsearch.test.junit.annotations.TestLogging;
import org.junit.AfterClass;
import org.junit.BeforeClass;
@ -21,12 +19,11 @@ import java.util.Map;
import static java.util.Collections.singletonMap;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.ESIntegTestCase.Scope.TEST;
import static org.hamcrest.Matchers.is;
@ClusterScope(scope = TEST)
public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
public static final String ROLES =
private static final String ROLES =
"role_a:\n" +
" cluster: [ all ]\n" +
"\n" +
@ -38,17 +35,17 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
" - names: 'someindex'\n" +
" privileges: [ all ]\n";
public static final String USERS =
private static final String USERS =
"user_a:" + USERS_PASSWD_HASHED + "\n" +
"user_b:" + USERS_PASSWD_HASHED + "\n" +
"user_c:" + USERS_PASSWD_HASHED + "\n";
public static final String USERS_ROLES =
private static final String USERS_ROLES =
"role_a:user_a\n" +
"role_b:user_b\n" +
"role_c:user_c\n";
static Path repositoryLocation;
private static Path repositoryLocation;
@BeforeClass
public static void setupRepostoryPath() {
@ -61,8 +58,8 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
}
@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal))
protected Settings nodeSettings() {
return Settings.builder().put(super.nodeSettings())
.put(NetworkModule.HTTP_ENABLED.getKey(), true)
.put("path.repo", repositoryLocation)
.build();
@ -83,6 +80,10 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
return super.configUsersRoles() + USERS_ROLES;
}
protected boolean resetNodeAfterTest() {
return true;
}
@TestLogging("org.elasticsearch.test.rest.client.http:TRACE")
public void testThatClusterPrivilegesWorkAsExpectedViaHttp() throws Exception {
// user_a can do all the things
@ -96,6 +97,7 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsAllowed("user_a", "GET", "/_nodes/infos");
assertAccessIsAllowed("user_a", "POST", "/_cluster/reroute");
assertAccessIsAllowed("user_a", "PUT", "/_cluster/settings", "{ \"transient\" : { \"search.default_search_timeout\": \"1m\" } }");
assertAccessIsAllowed("user_a", "PUT", "/_cluster/settings", "{ \"transient\" : { \"search.default_search_timeout\": null } }");
// user_b can do monitoring
assertAccessIsAllowed("user_b", "GET", "/_cluster/state");
@ -169,17 +171,10 @@ public class ClusterPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsAllowed("user_a", "DELETE", "/_snapshot/my-repo");
}
private void waitForSnapshotToFinish(String repo, String snapshot) {
SnapshotsStatusResponse snapshotsStatusResponse;
int i = 0;
do {
snapshotsStatusResponse = client().admin().cluster().prepareSnapshotStatus(repo).setSnapshots(snapshot).get();
try {
Thread.sleep(200L);
} catch (InterruptedException e) {}
i++;
if (i >= 20) { throw new ElasticsearchException("Snapshot should have been successfully created after four seconds, was " +
snapshotsStatusResponse.getSnapshots().get(0).getState()); }
} while (snapshotsStatusResponse.getSnapshots().get(0).getState() != SnapshotsInProgress.State.SUCCESS);
private void waitForSnapshotToFinish(String repo, String snapshot) throws Exception {
assertBusy(() -> {
SnapshotsStatusResponse response = client().admin().cluster().prepareSnapshotStatus(repo).setSnapshots(snapshot).get();
assertThat(response.getSnapshots().get(0).getState(), is(SnapshotsInProgress.State.SUCCESS));
});
}
}

View File

@ -10,25 +10,17 @@ import org.elasticsearch.client.ResponseException;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.transport.Netty4Plugin;
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
import org.junit.Before;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Locale;
import java.util.Map;
import static java.util.Collections.singletonMap;
import static org.apache.lucene.util.LuceneTestCase.BadApple;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
import static org.hamcrest.Matchers.is;
//test is just too slow, please fix it to not be sleep-based
@BadApple(bugUrl = "https://github.com/elastic/x-plugins/issues/1007")
@ESIntegTestCase.ClusterScope(maxNumDataNodes = 2)
public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
private String jsonDoc = "{ \"name\" : \"elasticsearch\", \"body\": \"foo bar\" }";
@ -128,19 +120,12 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
"index_a_role:u13\n";
@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder().put(super.nodeSettings(nodeOrdinal))
protected Settings nodeSettings() {
return Settings.builder().put(super.nodeSettings())
.put(NetworkModule.HTTP_ENABLED.getKey(), true)
.build();
}
@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
ArrayList<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins());
plugins.add(Netty4Plugin.class); // for http
return plugins;
}
@Override
protected String configRoles() {
return super.configRoles() + "\n" + ROLES;
@ -156,17 +141,6 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
return super.configUsersRoles() + USERS_ROLES;
}
// we reduce the number of shards and replicas to help speed up this test since that is not the focus of this test
@Override
public int maximumNumberOfReplicas() {
return 1;
}
@Override
public int maximumNumberOfShards() {
return 2;
}
@Before
public void insertBaseDocumentsAsAdmin() throws Exception {
// indices: a,b,c,abc
@ -452,7 +426,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsAllowed(user, "DELETE", "/" + index);
assertUserIsAllowed(user, "create_index", index);
// wait until index ready, but as admin
client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get();
assertNoTimeout(client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get());
assertAccessIsAllowed(user, "POST", "/" + index + "/_refresh");
assertAccessIsAllowed(user, "GET", "/" + index + "/_analyze", "{ \"text\" : \"test\" }");
assertAccessIsAllowed(user, "POST", "/" + index + "/_flush");
@ -463,7 +437,7 @@ public class IndexPrivilegeTests extends AbstractPrivilegeTestCase {
assertAccessIsAllowed(user, "POST", "/" + index + "/_cache/clear");
// indexing a document to have the mapping available, and wait for green state to make sure index is created
assertAccessIsAllowed("admin", "PUT", "/" + index + "/foo/1", jsonDoc, refreshParams);
client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get();
assertNoTimeout(client().admin().cluster().prepareHealth(index).setWaitForGreenStatus().get());
assertAccessIsAllowed(user, "GET", "/" + index + "/_mapping/foo/field/name");
assertAccessIsAllowed(user, "GET", "/" + index + "/_settings");
} else {

View File

@ -8,7 +8,6 @@ package org.elasticsearch.test;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.analysis.common.CommonAnalysisPlugin;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.network.NetworkModule;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.SecureSettings;
@ -29,14 +28,10 @@ import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail;
import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.security.test.SecurityTestUtils;
import java.io.IOException;
import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.FileSystemNotFoundException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
@ -110,13 +105,13 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas
this.usePEM = randomBoolean();
}
private Path nodePath(final Path parentFolder, final String subfolderPrefix, final int nodeOrdinal) {
Path nodePath(final int nodeOrdinal) {
return parentFolder.resolve(subfolderPrefix + "-" + nodeOrdinal);
}
@Override
public Settings nodeSettings(int nodeOrdinal) {
final Path home = nodePath(parentFolder, subfolderPrefix, nodeOrdinal);
final Path home = nodePath(nodeOrdinal);
final Path xpackConf = home.resolve("config").resolve(XPackField.NAME);
try {
Files.createDirectories(xpackConf);
@ -146,7 +141,7 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas
@Override
public Path nodeConfigPath(int nodeOrdinal) {
return nodePath(parentFolder, subfolderPrefix, nodeOrdinal).resolve("config");
return nodePath(nodeOrdinal).resolve("config");
}
@Override

View File

@ -0,0 +1,258 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.test;
import org.apache.http.HttpHost;
import org.elasticsearch.action.admin.cluster.node.info.NodeInfo;
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.common.network.NetworkAddress;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.license.LicenseService;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.xpack.security.LocalStateSecurity;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.rules.ExternalResource;
import java.net.InetSocketAddress;
import java.nio.file.Path;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.stream.Collectors;
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING;
import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
import static org.hamcrest.core.IsCollectionContaining.hasItem;
/**
* A test that starts a single node with security enabled. This test case allows for customization
* of users and roles that the cluster starts with. If an integration test is needed to test but
* multiple nodes are not needed, then this class should be favored over
* {@link SecurityIntegTestCase} due to simplicity and improved speed from not needing to start
* multiple nodes and wait for the cluster to form.
*/
public abstract class SecuritySingleNodeTestCase extends ESSingleNodeTestCase {
private static SecuritySettingsSource SECURITY_DEFAULT_SETTINGS = null;
private static CustomSecuritySettingsSource customSecuritySettingsSource = null;
private static RestClient restClient = null;
private static SecureString BOOTSTRAP_PASSWORD = null;
@BeforeClass
public static void generateBootstrapPassword() {
BOOTSTRAP_PASSWORD = TEST_PASSWORD_SECURE_STRING.clone();
}
@BeforeClass
public static void initDefaultSettings() {
if (SECURITY_DEFAULT_SETTINGS == null) {
SECURITY_DEFAULT_SETTINGS =
new SecuritySettingsSource(1, randomBoolean(), createTempDir(), ESIntegTestCase.Scope.SUITE);
}
}
/**
* Set the static default settings to null to prevent a memory leak. The test framework also checks for memory leaks
* and computes the size, this can cause issues when running with the security manager as it tries to do reflection
* into protected sun packages.
*/
@AfterClass
public static void destroyDefaultSettings() {
SECURITY_DEFAULT_SETTINGS = null;
customSecuritySettingsSource = null;
if (BOOTSTRAP_PASSWORD != null) {
BOOTSTRAP_PASSWORD.close();
BOOTSTRAP_PASSWORD = null;
}
if (restClient != null) {
IOUtils.closeWhileHandlingException(restClient);
restClient = null;
}
}
@Rule
//Rules are the only way to have something run before the before (final) method inherited from ESSingleNodeTestCase
public ExternalResource externalResource = new ExternalResource() {
@Override
protected void before() {
if (customSecuritySettingsSource == null) {
customSecuritySettingsSource =
new CustomSecuritySettingsSource(transportSSLEnabled(), createTempDir(), ESIntegTestCase.Scope.SUITE);
}
}
};
@Before
//before methods from the superclass are run before this, which means that the current cluster is ready to go
public void assertXPackIsInstalled() {
doAssertXPackIsInstalled();
}
private void doAssertXPackIsInstalled() {
NodesInfoResponse nodeInfos = client().admin().cluster().prepareNodesInfo().clear().setPlugins(true).get();
for (NodeInfo nodeInfo : nodeInfos.getNodes()) {
// TODO: disable this assertion for now, due to random runs with mock plugins. perhaps run without mock plugins?
// assertThat(nodeInfo.getPlugins().getInfos(), hasSize(2));
Collection<String> pluginNames =
nodeInfo.getPlugins().getPluginInfos().stream().map(PluginInfo::getClassname).collect(Collectors.toList());
assertThat("plugin [" + LocalStateSecurity.class.getName() + "] not found in [" + pluginNames + "]", pluginNames,
hasItem(LocalStateSecurity.class.getName()));
}
}
@Override
protected Settings nodeSettings() {
Settings.Builder builder = Settings.builder().put(super.nodeSettings());
Settings customSettings = customSecuritySettingsSource.nodeSettings(0);
builder.put(customSettings, false); // handle secure settings separately
builder.put(LicenseService.SELF_GENERATED_LICENSE_TYPE.getKey(), "trial");
builder.put("transport.type", "security4");
builder.put("path.home", customSecuritySettingsSource.nodePath(0));
Settings.Builder customBuilder = Settings.builder().put(customSettings);
if (customBuilder.getSecureSettings() != null) {
SecuritySettingsSource.addSecureSettings(builder, secureSettings ->
secureSettings.merge((MockSecureSettings) customBuilder.getSecureSettings()));
}
if (builder.getSecureSettings() == null) {
builder.setSecureSettings(new MockSecureSettings());
}
((MockSecureSettings) builder.getSecureSettings()).setString("bootstrap.password", BOOTSTRAP_PASSWORD.toString());
return builder.build();
}
@Override
protected Collection<Class<? extends Plugin>> getPlugins() {
return customSecuritySettingsSource.nodePlugins();
}
/**
* Allows to override the users config file
*/
protected String configUsers() {
return SECURITY_DEFAULT_SETTINGS.configUsers();
}
/**
* Allows to override the users_roles config file
*/
protected String configUsersRoles() {
return SECURITY_DEFAULT_SETTINGS.configUsersRoles();
}
/**
* Allows to override the roles config file
*/
protected String configRoles() {
return SECURITY_DEFAULT_SETTINGS.configRoles();
}
/**
* Allows to override the node client username
*/
protected String nodeClientUsername() {
return SECURITY_DEFAULT_SETTINGS.nodeClientUsername();
}
/**
* Allows to override the node client password (used while sending requests to the test node)
*/
protected SecureString nodeClientPassword() {
return SECURITY_DEFAULT_SETTINGS.nodeClientPassword();
}
/**
* Allows to control whether ssl key information is auto generated or not on the transport layer
*/
protected boolean transportSSLEnabled() {
return randomBoolean();
}
private class CustomSecuritySettingsSource extends SecuritySettingsSource {
private CustomSecuritySettingsSource(boolean sslEnabled, Path configDir, ESIntegTestCase.Scope scope) {
super(1, sslEnabled, configDir, scope);
}
@Override
protected String configUsers() {
return SecuritySingleNodeTestCase.this.configUsers();
}
@Override
protected String configUsersRoles() {
return SecuritySingleNodeTestCase.this.configUsersRoles();
}
@Override
protected String configRoles() {
return SecuritySingleNodeTestCase.this.configRoles();
}
@Override
protected String nodeClientUsername() {
return SecuritySingleNodeTestCase.this.nodeClientUsername();
}
@Override
protected SecureString nodeClientPassword() {
return SecuritySingleNodeTestCase.this.nodeClientPassword();
}
}
@Override
public Client client() {
Map<String, String> headers = Collections.singletonMap("Authorization",
basicAuthHeaderValue(nodeClientUsername(), nodeClientPassword()));
// we need to wrap node clients because we do not specify a user for nodes and all requests will use the system
// user. This is ok for internal n2n stuff but the test framework does other things like wiping indices, repositories, etc
// that the system user cannot do. so we wrap the node client with a user that can do these things since the client() calls
// are all using a node client
return super.client().filterWithHeader(headers);
}
protected boolean isTransportSSLEnabled() {
return customSecuritySettingsSource.isSslEnabled();
}
/**
* Returns an instance of {@link RestClient} pointing to the current node.
* Creates a new client if the method is invoked for the first time in the context of the current test scope.
* The returned client gets automatically closed when needed, it shouldn't be closed as part of tests otherwise
* it cannot be reused by other tests anymore.
*/
protected RestClient getRestClient() {
return getRestClient(client());
}
private static synchronized RestClient getRestClient(Client client) {
if (restClient == null) {
restClient = createRestClient(client);
}
return restClient;
}
private static RestClient createRestClient(Client client) {
NodesInfoResponse nodesInfoResponse = client.admin().cluster().prepareNodesInfo().get();
assertFalse(nodesInfoResponse.hasFailures());
assertEquals(nodesInfoResponse.getNodes().size(), 1);
NodeInfo node = nodesInfoResponse.getNodes().get(0);
assertNotNull(node.getHttp());
TransportAddress publishAddress = node.getHttp().address().publishAddress();
InetSocketAddress address = publishAddress.address();
final HttpHost host = new HttpHost(NetworkAddress.format(address.getAddress()), address.getPort(), "http");
return RestClient.builder(host).build();
}
}