From b3b6de74616f5b418bf608ec9c648cc8e95a8b1c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 3 Jan 2024 15:47:37 -0500 Subject: [PATCH] Add setting to delay initialization in order to write a test with clearer setup and tearDown Signed-off-by: Craig Perkins --- .../security/ConfigurationFiles.java | 1 - .../SecurityConfigurationBootstrapTests.java | 91 +++++++++++++------ .../test/framework/cluster/LocalCluster.java | 4 +- .../security/OpenSearchSecurityPlugin.java | 8 ++ .../ConfigurationRepository.java | 13 ++- .../security/support/ConfigConstants.java | 2 + 6 files changed, 85 insertions(+), 34 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/ConfigurationFiles.java b/src/integrationTest/java/org/opensearch/security/ConfigurationFiles.java index dbd3265dfe..f3b7613aa1 100644 --- a/src/integrationTest/java/org/opensearch/security/ConfigurationFiles.java +++ b/src/integrationTest/java/org/opensearch/security/ConfigurationFiles.java @@ -31,7 +31,6 @@ public static Path createConfigurationDirectory() { String[] configurationFiles = { "config.yml", "action_groups.yml", - "config.yml", "internal_users.yml", "nodes_dn.yml", "roles.yml", diff --git a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationBootstrapTests.java b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationBootstrapTests.java index 8225c686d2..5a569aa03f 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationBootstrapTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationBootstrapTests.java @@ -18,14 +18,15 @@ import org.apache.commons.io.FileUtils; import org.awaitility.Awaitility; import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.client.Client; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.support.ConfigHelper; import org.opensearch.test.framework.TestSecurityConfig.User; import org.opensearch.test.framework.cluster.ClusterManager; import org.opensearch.test.framework.cluster.ContextHeaderDecoratorClient; @@ -33,8 +34,11 @@ import org.opensearch.test.framework.cluster.TestRestClient; import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.security.configuration.ConfigurationRepository.DEFAULT_CONFIG_VERSION; +import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX; import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX; import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; +import static org.opensearch.security.support.ConfigConstants.SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @@ -46,7 +50,7 @@ public class SecurityConfigurationBootstrapTests { private static final User USER_ADMIN = new User("admin").roles(ALL_ACCESS); @ClassRule - public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) .loadConfigurationIntoIndex(false) .defaultConfigurationInitDirectory(configurationFolder.toString()) .nodeSettings( @@ -54,43 +58,72 @@ public class SecurityConfigurationBootstrapTests { SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, - true + true, + SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS, + 5 ) ) .build(); - @BeforeClass - public static void runConfigUpdateRequestsInBgThread() { - - Client client = new ContextHeaderDecoratorClient( - cluster.getInternalNodeClient(), - Map.of(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true") - ); - Runnable r = new Runnable() { - public void run() { - long t = System.currentTimeMillis(); - long end = t + 10000; - while (System.currentTimeMillis() < end) { - cluster.triggerConfigurationReloadForSingleCType(client, CType.CONFIG, true); - try { - Thread.sleep(50); - } catch (InterruptedException e) { - break; - } - } - } - }; - - new Thread(r).start(); - } - @AfterClass public static void cleanConfigurationDirectory() throws IOException { FileUtils.deleteDirectory(configurationFolder.toFile()); } @Test - public void shouldStillLoadSecurityConfigDuringBootstrapAndActiveConfigUpdateRequests() { + public void shouldStillLoadSecurityConfigDuringBootstrapAndActiveConfigUpdateRequests() throws Exception { + cluster.getInternalNodeClient() + .admin() + .cluster() + .health(new ClusterHealthRequest(OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX).waitForGreenStatus()) + .actionGet(); + Client internalNodeClient = new ContextHeaderDecoratorClient( + cluster.getInternalNodeClient(), + Map.of(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true") + ); + String cd = System.getProperty("security.default_init.dir") + "/"; + ConfigHelper.uploadFile( + internalNodeClient, + cd + "action_groups.yml", + OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX, + CType.ACTIONGROUPS, + DEFAULT_CONFIG_VERSION + ); + ConfigHelper.uploadFile( + internalNodeClient, + cd + "config.yml", + OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX, + CType.CONFIG, + DEFAULT_CONFIG_VERSION + ); + ConfigHelper.uploadFile( + internalNodeClient, + cd + "roles.yml", + OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX, + CType.ROLES, + DEFAULT_CONFIG_VERSION + ); + ConfigHelper.uploadFile( + internalNodeClient, + cd + "tenants.yml", + OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX, + CType.TENANTS, + DEFAULT_CONFIG_VERSION + ); + long t = System.currentTimeMillis(); + long end = t + 10000; + while (System.currentTimeMillis() < end) { + cluster.triggerConfigurationReloadForCTypes( + internalNodeClient, + List.of(CType.ACTIONGROUPS, CType.CONFIG, CType.ROLES, CType.TENANTS), + true + ); + try { + Thread.sleep(100); + } catch (InterruptedException e) { + break; + } + } try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { Awaitility.await().alias("Load default configuration").until(() -> client.getAuthInfo().getStatusCode(), equalTo(200)); diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java index 081ba602c2..592c981c57 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java @@ -297,10 +297,10 @@ private static void triggerConfigurationReload(Client client) { } } - public void triggerConfigurationReloadForSingleCType(Client client, CType cType, boolean ignoreFailures) { + public void triggerConfigurationReloadForCTypes(Client client, List cTypes, boolean ignoreFailures) { ConfigUpdateResponse configUpdateResponse = client.execute( ConfigUpdateAction.INSTANCE, - new ConfigUpdateRequest(new String[] { cType.toLCString() }) + new ConfigUpdateRequest(cTypes.stream().map(CType::toLCString).toArray(String[]::new)) ).actionGet(); if (!ignoreFailures && configUpdateResponse.hasFailures()) { throw new RuntimeException("ConfigUpdateResponse produced failures: " + configUpdateResponse.failures()); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 3c04816c32..0a39e73497 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1784,6 +1784,14 @@ public List> getSettings() { Property.Filtered ) ); + settings.add( + Setting.intSetting( + ConfigConstants.SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS, + 0, + Property.NodeScope, + Property.Filtered + ) + ); // system integration settings.add( diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java index efb5ac539d..25b1da8248 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -93,7 +93,7 @@ public class ConfigurationRepository { private final AuditLog auditLog; private final ThreadPool threadPool; private DynamicConfigFactory dynamicConfigFactory; - private static final int DEFAULT_CONFIG_VERSION = 2; + public static final int DEFAULT_CONFIG_VERSION = 2; private CompletableFuture bgThreadRunner = new CompletableFuture<>(); private final Thread bgThread; private final AtomicBoolean installDefaultConfig = new AtomicBoolean(); @@ -147,6 +147,15 @@ private ConfigurationRepository( createSecurityIndexIfAbsent(); waitForSecurityIndexToBeAtLeastYellow(); + int initializationDelay = settings.getAsInt( + ConfigConstants.SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS, + 0 + ); + if (initializationDelay > 0) { + LOGGER.info("Delay initialization for {} seconds", initializationDelay); + TimeUnit.SECONDS.sleep(initializationDelay); + } + ConfigHelper.uploadFile(client, cd + "config.yml", securityIndex, CType.CONFIG, DEFAULT_CONFIG_VERSION); ConfigHelper.uploadFile(client, cd + "roles.yml", securityIndex, CType.ROLES, DEFAULT_CONFIG_VERSION); ConfigHelper.uploadFile( @@ -396,7 +405,7 @@ public boolean reloadConfiguration(Collection configTypes) throws ConfigU LOCK.unlock(); } } else { - throw new ConfigUpdateAlreadyInProgressException("A config update is already imn progress"); + throw new ConfigUpdateAlreadyInProgressException("A config update is already in progress"); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 2ed5e4417f..b6314f3dc1 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -281,6 +281,8 @@ public enum RolesMappingResolution { // Illegal Opcodes from here on public static final String SECURITY_UNSUPPORTED_DISABLE_REST_AUTH_INITIALLY = "plugins.security.unsupported.disable_rest_auth_initially"; + public static final String SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS = + "plugins.security.unsupported.delay_initialization_seconds"; public static final String SECURITY_UNSUPPORTED_DISABLE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.disable_intertransport_auth_initially"; public static final String SECURITY_UNSUPPORTED_PASSIVE_INTERTRANSPORT_AUTH_INITIALLY =